Skip to content

Commit 161cca2

Browse files
author
Kostya Kortchinsky
committed
[scudo][standalone] Android related improvements
Summary: This changes a few things to improve memory footprint and performances on Android, and fixes a test compilation error: - add `stdlib.h` to `wrappers_c_test.cc` to address https://bugs.llvm.org/show_bug.cgi?id=42810 - change Android size class maps, based on benchmarks, to improve performances and lower the Svelte memory footprint. Also change the 32-bit region size for said configuration - change the `reallocate` logic to reallocate in place for sizes larger than the original chunk size, when they still fit in the same block. This addresses patterns from `memory_replay` dumps like the following: ``` 202: realloc 0xb48fd000 0xb4930650 12352 202: realloc 0xb48fd000 0xb48fd000 12420 202: realloc 0xb48fd000 0xb48fd000 12492 202: realloc 0xb48fd000 0xb48fd000 12564 202: realloc 0xb48fd000 0xb48fd000 12636 202: realloc 0xb48fd000 0xb48fd000 12708 202: realloc 0xb48fd000 0xb48fd000 12780 202: realloc 0xb48fd000 0xb48fd000 12852 202: realloc 0xb48fd000 0xb48fd000 12924 202: realloc 0xb48fd000 0xb48fd000 12996 202: realloc 0xb48fd000 0xb48fd000 13068 202: realloc 0xb48fd000 0xb48fd000 13140 202: realloc 0xb48fd000 0xb48fd000 13212 202: realloc 0xb48fd000 0xb48fd000 13284 202: realloc 0xb48fd000 0xb48fd000 13356 202: realloc 0xb48fd000 0xb48fd000 13428 202: realloc 0xb48fd000 0xb48fd000 13500 202: realloc 0xb48fd000 0xb48fd000 13572 202: realloc 0xb48fd000 0xb48fd000 13644 202: realloc 0xb48fd000 0xb48fd000 13716 202: realloc 0xb48fd000 0xb48fd000 13788 ... ``` In this situation we were deallocating the old chunk, and allocating a new one for every single one of those, but now we can keep the same chunk (we just updated the header), which saves some heap operations. Reviewers: hctim, morehouse, vitalybuka, eugenis, cferris, rengolin Reviewed By: morehouse Subscribers: srhines, delcypher, #sanitizers, llvm-commits Tags: #llvm, #sanitizers Differential Revision: https://reviews.llvm.org/D67293 llvm-svn: 371628
1 parent 5957a61 commit 161cca2

File tree

5 files changed

+44
-15
lines changed

5 files changed

+44
-15
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ struct AndroidSvelteConfig {
5353
// 512MB regions
5454
typedef SizeClassAllocator64<SizeClassMap, 29U> Primary;
5555
#else
56-
// 256KB regions
57-
typedef SizeClassAllocator32<SizeClassMap, 18U> Primary;
56+
// 64KB regions
57+
typedef SizeClassAllocator32<SizeClassMap, 16U> Primary;
5858
#endif
5959
template <class A>
6060
using TSDRegistryT = TSDRegistrySharedT<A, 1U>; // Shared, only 1 TSD.

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

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -311,18 +311,30 @@ template <class Params> class Allocator {
311311
OldHeader.Origin, Chunk::Origin::Malloc);
312312
}
313313

314-
const uptr OldSize = getSize(OldPtr, &OldHeader);
315-
// If the new size is identical to the old one, or lower but within an
316-
// acceptable range, we just keep the old chunk, and update its header.
317-
if (UNLIKELY(NewSize == OldSize))
318-
return OldPtr;
319-
if (NewSize < OldSize) {
320-
const uptr Delta = OldSize - NewSize;
321-
if (Delta < (SizeClassMap::MaxSize / 2)) {
314+
void *BlockBegin = getBlockBegin(OldPtr, &OldHeader);
315+
uptr BlockEnd;
316+
uptr OldSize;
317+
const uptr ClassId = OldHeader.ClassId;
318+
if (LIKELY(ClassId)) {
319+
BlockEnd = reinterpret_cast<uptr>(BlockBegin) +
320+
SizeClassMap::getSizeByClassId(ClassId);
321+
OldSize = OldHeader.SizeOrUnusedBytes;
322+
} else {
323+
BlockEnd = SecondaryT::getBlockEnd(BlockBegin);
324+
OldSize = BlockEnd -
325+
(reinterpret_cast<uptr>(OldPtr) + OldHeader.SizeOrUnusedBytes);
326+
}
327+
// If the new chunk still fits in the previously allocated block (with a
328+
// reasonable delta), we just keep the old block, and update the chunk
329+
// header to reflect the size change.
330+
if (reinterpret_cast<uptr>(OldPtr) + NewSize <= BlockEnd) {
331+
const uptr Delta =
332+
OldSize < NewSize ? NewSize - OldSize : OldSize - NewSize;
333+
if (Delta <= SizeClassMap::MaxSize / 2) {
322334
Chunk::UnpackedHeader NewHeader = OldHeader;
323335
NewHeader.SizeOrUnusedBytes =
324-
(OldHeader.ClassId ? NewHeader.SizeOrUnusedBytes - Delta
325-
: NewHeader.SizeOrUnusedBytes + Delta) &
336+
(ClassId ? NewSize
337+
: BlockEnd - (reinterpret_cast<uptr>(OldPtr) + NewSize)) &
326338
Chunk::SizeOrUnusedBytesMask;
327339
Chunk::compareExchangeHeader(Cookie, OldPtr, &NewHeader, &OldHeader);
328340
return OldPtr;
@@ -335,6 +347,7 @@ template <class Params> class Allocator {
335347
// are currently unclear.
336348
void *NewPtr = allocate(NewSize, Chunk::Origin::Malloc, Alignment);
337349
if (NewPtr) {
350+
const uptr OldSize = getSize(OldPtr, &OldHeader);
338351
memcpy(NewPtr, OldPtr, Min(NewSize, OldSize));
339352
quarantineOrDeallocateChunk(OldPtr, &OldHeader, OldSize);
340353
}

compiler-rt/lib/scudo/standalone/size_class_map.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,11 @@ typedef SizeClassMap<3, 5, 8, 17, 8, 10> DefaultSizeClassMap;
137137

138138
// TODO(kostyak): further tune class maps for Android & Fuchsia.
139139
#if SCUDO_WORDSIZE == 64U
140-
typedef SizeClassMap<3, 5, 8, 15, 8, 10> SvelteSizeClassMap;
140+
typedef SizeClassMap<4, 4, 8, 14, 4, 10> SvelteSizeClassMap;
141141
typedef SizeClassMap<3, 5, 8, 17, 14, 14> AndroidSizeClassMap;
142142
#else
143-
typedef SizeClassMap<3, 4, 7, 15, 8, 10> SvelteSizeClassMap;
144-
typedef SizeClassMap<3, 4, 7, 17, 14, 14> AndroidSizeClassMap;
143+
typedef SizeClassMap<4, 3, 7, 14, 5, 10> SvelteSizeClassMap;
144+
typedef SizeClassMap<3, 5, 8, 17, 14, 14> AndroidSizeClassMap;
145145
#endif
146146

147147
} // namespace scudo

compiler-rt/lib/scudo/standalone/tests/combined_test.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,21 @@ template <class Config> static void testAllocator() {
9797
}
9898
Allocator->deallocate(P, Origin);
9999

100+
// Check that reallocating a chunk to a slightly smaller or larger size
101+
// returns the same chunk. This requires that all the sizes we iterate on use
102+
// the same block size, but that should be the case for 2048 with our default
103+
// class size maps.
104+
P = Allocator->allocate(DataSize, Origin);
105+
memset(P, Marker, DataSize);
106+
for (scudo::sptr Delta = -32; Delta < 32; Delta += 8) {
107+
const scudo::uptr NewSize = DataSize + Delta;
108+
void *NewP = Allocator->reallocate(P, NewSize);
109+
EXPECT_EQ(NewP, P);
110+
for (scudo::uptr I = 0; I < scudo::Min(DataSize, NewSize); I++)
111+
EXPECT_EQ((reinterpret_cast<char *>(NewP))[I], Marker);
112+
}
113+
Allocator->deallocate(P, Origin);
114+
100115
// Allocates a bunch of chunks, then iterate over all the chunks, ensuring
101116
// they are the ones we allocated. This requires the allocator to not have any
102117
// other allocated chunk at this point (eg: won't work with the Quarantine).

compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
#include <limits.h>
1414
#include <malloc.h>
15+
#include <stdlib.h>
1516
#include <unistd.h>
1617

1718
extern "C" {

0 commit comments

Comments
 (0)