Skip to content

Commit 993e3c9

Browse files
author
Kostya Kortchinsky
committed
[scudo][standalone] Secondary & general other improvements
Summary: This CL changes multiple things to improve performance (notably on Android).We introduce a cache class for the Secondary that is taking care of this mechanism now. The changes: - change the Secondary "freelist" to an array. By keeping free secondary blocks linked together through their headers, we were keeping a page per block, which isn't great. Also we know touch less pages when walking the new "freelist". - fix an issue with the freelist getting full: if the pattern is an ever increasing size malloc then free, the freelist would fill up and entries would not be used. So now we empty the list if we get to many "full" events; - use the global release to os interval option for the secondary: it was too costly to release all the time, particularly for pattern that are malloc(X)/free(X)/malloc(X). Now the release will only occur after the selected interval, when going through the deallocate path; - allow release of the `BatchClassId` class: it is releasable, we just have to make sure we don't mark the batches containing batches pointers as free. - change the default release interval to 1s for Android to match the current Bionic allocator configuration. A patch is coming up to allow changing it through `mallopt`. - lower the smallest class that can be released to `PageSize/64`. Reviewers: cferris, pcc, eugenis, morehouse, hctim Subscribers: phosek, #sanitizers, llvm-commits Tags: #sanitizers, #llvm Differential Revision: https://reviews.llvm.org/D73507
1 parent 731b140 commit 993e3c9

File tree

9 files changed

+240
-83
lines changed

9 files changed

+240
-83
lines changed

‎compiler-rt/lib/scudo/standalone/allocator_config.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ struct DefaultConfig {
3232
// 512KB regions
3333
typedef SizeClassAllocator32<SizeClassMap, 19U> Primary;
3434
#endif
35-
typedef MapAllocator<> Secondary;
35+
typedef MapAllocator<MapAllocatorCache<>> Secondary;
3636
template <class A> using TSDRegistryT = TSDRegistryExT<A>; // Exclusive
3737
};
3838

@@ -47,7 +47,7 @@ struct AndroidConfig {
4747
// 512KB regions
4848
typedef SizeClassAllocator32<SizeClassMap, 19U> Primary;
4949
#endif
50-
typedef MapAllocator<> Secondary;
50+
typedef MapAllocator<MapAllocatorCache<>> Secondary;
5151
template <class A>
5252
using TSDRegistryT = TSDRegistrySharedT<A, 2U>; // Shared, max 2 TSDs.
5353
};
@@ -61,7 +61,7 @@ struct AndroidSvelteConfig {
6161
// 64KB regions
6262
typedef SizeClassAllocator32<SizeClassMap, 16U> Primary;
6363
#endif
64-
typedef MapAllocator<0U> Secondary;
64+
typedef MapAllocator<MapAllocatorCache<4U, 1UL << 18>> Secondary;
6565
template <class A>
6666
using TSDRegistryT = TSDRegistrySharedT<A, 1U>; // Shared, only 1 TSD.
6767
};
@@ -70,7 +70,7 @@ struct AndroidSvelteConfig {
7070
struct FuchsiaConfig {
7171
// 1GB Regions
7272
typedef SizeClassAllocator64<DefaultSizeClassMap, 30U> Primary;
73-
typedef MapAllocator<0U> Secondary;
73+
typedef MapAllocator<MapAllocatorNoCache> Secondary;
7474
template <class A>
7575
using TSDRegistryT = TSDRegistrySharedT<A, 8U>; // Shared, max 8 TSDs.
7676
};

‎compiler-rt/lib/scudo/standalone/combined.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,9 @@ class Allocator {
141141
static_cast<u32>(getFlags()->quarantine_max_chunk_size);
142142

143143
Stats.initLinkerInitialized();
144-
Primary.initLinkerInitialized(getFlags()->release_to_os_interval_ms);
145-
Secondary.initLinkerInitialized(&Stats);
144+
const s32 ReleaseToOsIntervalMs = getFlags()->release_to_os_interval_ms;
145+
Primary.initLinkerInitialized(ReleaseToOsIntervalMs);
146+
Secondary.initLinkerInitialized(&Stats, ReleaseToOsIntervalMs);
146147

147148
Quarantine.init(
148149
static_cast<uptr>(getFlags()->quarantine_size_kb << 10),

‎compiler-rt/lib/scudo/standalone/flags.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,6 @@ SCUDO_FLAG(bool, may_return_null, true,
4545
"returning NULL in otherwise non-fatal error scenarios, eg: OOM, "
4646
"invalid allocation alignments, etc.")
4747

48-
SCUDO_FLAG(int, release_to_os_interval_ms, 5000,
48+
SCUDO_FLAG(int, release_to_os_interval_ms, SCUDO_ANDROID ? 1000 : 5000,
4949
"Interval (in milliseconds) at which to attempt release of unused "
5050
"memory to the OS. Negative values disable the feature.")

‎compiler-rt/lib/scudo/standalone/primary32.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ template <class SizeClassMapT, uptr RegionSizeLog> class SizeClassAllocator32 {
7474
Sci->RandState = getRandomU32(&Seed);
7575
// See comment in the 64-bit primary about releasing smaller size classes.
7676
Sci->CanRelease = (ReleaseToOsInterval >= 0) &&
77-
(I != SizeClassMap::BatchClassId) &&
78-
(getSizeByClassId(I) >= (PageSize / 32));
77+
(getSizeByClassId(I) >= (PageSize / 64));
7978
}
8079
ReleaseToOsIntervalMs = ReleaseToOsInterval;
8180
}
@@ -385,7 +384,7 @@ template <class SizeClassMapT, uptr RegionSizeLog> class SizeClassAllocator32 {
385384
if (IntervalMs < 0)
386385
return 0;
387386
if (Sci->ReleaseInfo.LastReleaseAtNs +
388-
static_cast<uptr>(IntervalMs) * 1000000ULL >
387+
static_cast<u64>(IntervalMs) * 1000000 >
389388
getMonotonicTime()) {
390389
return 0; // Memory was returned recently.
391390
}

‎compiler-rt/lib/scudo/standalone/primary64.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ class SizeClassAllocator64 {
8787
// limit is mostly arbitrary and based on empirical observations.
8888
// TODO(kostyak): make the lower limit a runtime option
8989
Region->CanRelease = (ReleaseToOsInterval >= 0) &&
90-
(I != SizeClassMap::BatchClassId) &&
91-
(getSizeByClassId(I) >= (PageSize / 32));
90+
(getSizeByClassId(I) >= (PageSize / 64));
9291
Region->RandState = getRandomU32(&Seed);
9392
}
9493
ReleaseToOsIntervalMs = ReleaseToOsInterval;
@@ -401,7 +400,7 @@ class SizeClassAllocator64 {
401400
if (IntervalMs < 0)
402401
return 0;
403402
if (Region->ReleaseInfo.LastReleaseAtNs +
404-
static_cast<uptr>(IntervalMs) * 1000000ULL >
403+
static_cast<u64>(IntervalMs) * 1000000 >
405404
getMonotonicTime()) {
406405
return 0; // Memory was returned recently.
407406
}

‎compiler-rt/lib/scudo/standalone/release.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,13 @@ releaseFreeMemoryToOS(const IntrusiveList<TransferBatchT> &FreeList, uptr Base,
200200
if (BlockSize <= PageSize && PageSize % BlockSize == 0) {
201201
// Each chunk affects one page only.
202202
for (const auto &It : FreeList) {
203-
for (u32 I = 0; I < It.getCount(); I++) {
203+
// If dealing with a TransferBatch, the first pointer of the batch will
204+
// point to the batch itself, we do not want to mark this for release as
205+
// the batch is in use, so skip the first entry.
206+
const bool IsTransferBatch =
207+
(It.getCount() != 0) &&
208+
(reinterpret_cast<uptr>(It.get(0)) == reinterpret_cast<uptr>(&It));
209+
for (u32 I = IsTransferBatch ? 1 : 0; I < It.getCount(); I++) {
204210
const uptr P = reinterpret_cast<uptr>(It.get(I));
205211
if (P >= Base && P < End)
206212
Counters.inc((P - Base) >> PageSizeLog);
@@ -209,7 +215,11 @@ releaseFreeMemoryToOS(const IntrusiveList<TransferBatchT> &FreeList, uptr Base,
209215
} else {
210216
// In all other cases chunks might affect more than one page.
211217
for (const auto &It : FreeList) {
212-
for (u32 I = 0; I < It.getCount(); I++) {
218+
// See TransferBatch comment above.
219+
const bool IsTransferBatch =
220+
(It.getCount() != 0) &&
221+
(reinterpret_cast<uptr>(It.get(0)) == reinterpret_cast<uptr>(&It));
222+
for (u32 I = IsTransferBatch ? 1 : 0; I < It.getCount(); I++) {
213223
const uptr P = reinterpret_cast<uptr>(It.get(I));
214224
if (P >= Base && P < End)
215225
Counters.incRange((P - Base) >> PageSizeLog,

0 commit comments

Comments
 (0)