[PATCH] D60787: [scudo][standalone] Introduce the Secondary allocator
Matt Morehouse via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 17 11:57:38 PDT 2019
morehouse added inline comments.
================
Comment at: lib/scudo/standalone/fuchsia.h:25
+ uint64_t VmoSize;
+} PlatformData;
+
----------------
Can we simplify to `struct PlatformData { ... }`?
================
Comment at: lib/scudo/standalone/linux.h:21
+ u8 Unused;
+} PlatformData;
+
----------------
Can we simplify to `struct PlatformData { ... }`?
================
Comment at: lib/scudo/standalone/secondary.h:64
+ // alignment, so that the frontend can align the user allocation. The hint
+ // parameter allows us to be able unmap spurious memory when dealing with
+ // larger (greater than a page) alignments.
----------------
"allows us to unmap" seems more readable.
================
Comment at: lib/scudo/standalone/secondary.h:76
+ MAP_NOACCESS | MAP_ALLOWNOMEM, &Data));
+ if (UNLIKELY(!MapBase))
+ return nullptr;
----------------
We're already on the slow path, and we just did a system call, so this path optimization is negligible. Same for below.
================
Comment at: lib/scudo/standalone/secondary.h:98
+ roundUpTo((Size - AlignmentHint), PageSize) +
+ PageSize;
+ DCHECK_LE(NewMapEnd, MapEnd);
----------------
When is `NewMapEnd < MapEnd`?
================
Comment at: lib/scudo/standalone/secondary.h:104
+ MapEnd = NewMapEnd;
+ }
+ }
----------------
So we potentially shrink the committed region for large alignments but did not allocate extra beforehand. Does this mean the actual region size may be less than the requested size?
Repository:
rCRT Compiler Runtime
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60787/new/
https://reviews.llvm.org/D60787
More information about the llvm-commits
mailing list