Skip to content

Commit 3f70987

Browse files
author
Kostya Kortchinsky
committed
[scudo][standalone] Small changes to the fastpath
There are a few things that I wanted to reorganize for a while: - the loop that incrementally goes through classes on failure looked horrible in assembly, mostly because of `LIKELY`/`UNLIKELY` within the loop. So remove those, we are already in an unlikely scenario - hooks are not used by default on Android/Fuchsia/etc so mark the tests for the existence of the weak functions as unlikely - mark of couple of conditions as likely/unlikely - in `reallocate`, the old size was computed again while we already have it in a variable. So just use the one we have. - remove the bitwise AND trick and use a logical AND, that has one less test by using a purposeful underflow when `Size` is 0 (I actually looked at the assembly of the previous code to steal that trick) - move the read of the options closer to where they are used, mark them as `const` Overall this makes things a tiny bit faster, but cleaner. Differential Revision: https://reviews.llvm.org/D92689
1 parent bdaeb82 commit 3f70987

File tree

1 file changed

+15
-19
lines changed

1 file changed

+15
-19
lines changed

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

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,6 @@ class Allocator {
277277
uptr Alignment = MinAlignment,
278278
bool ZeroContents = false) {
279279
initThreadMaybe();
280-
Options Options = Primary.Options.load();
281280

282281
#ifdef GWP_ASAN_HOOKS
283282
if (UNLIKELY(GuardedAlloc.shouldSample())) {
@@ -286,6 +285,7 @@ class Allocator {
286285
}
287286
#endif // GWP_ASAN_HOOKS
288287

288+
const Options Options = Primary.Options.load();
289289
const FillContentsMode FillContents = ZeroContents ? ZeroFill
290290
: TSDRegistry.getDisableMemInit()
291291
? NoFill
@@ -296,7 +296,7 @@ class Allocator {
296296
return nullptr;
297297
reportAlignmentTooBig(Alignment, MaxAlignment);
298298
}
299-
if (UNLIKELY(Alignment < MinAlignment))
299+
if (Alignment < MinAlignment)
300300
Alignment = MinAlignment;
301301

302302
// If the requested size happens to be 0 (more common than you might think),
@@ -331,12 +331,9 @@ class Allocator {
331331
// larger class until it fits. If it fails to fit in the largest class,
332332
// fallback to the Secondary.
333333
if (UNLIKELY(!Block)) {
334-
while (ClassId < SizeClassMap::LargestClassId) {
334+
while (ClassId < SizeClassMap::LargestClassId && !Block)
335335
Block = TSD->Cache.allocate(++ClassId);
336-
if (LIKELY(Block))
337-
break;
338-
}
339-
if (UNLIKELY(!Block))
336+
if (!Block)
340337
ClassId = 0;
341338
}
342339
if (UnlockRequired)
@@ -467,7 +464,7 @@ class Allocator {
467464
Chunk::SizeOrUnusedBytesMask;
468465
Chunk::storeHeader(Cookie, Ptr, &Header);
469466

470-
if (&__scudo_allocate_hook)
467+
if (UNLIKELY(&__scudo_allocate_hook))
471468
__scudo_allocate_hook(TaggedPtr, Size);
472469

473470
return TaggedPtr;
@@ -482,7 +479,6 @@ class Allocator {
482479
// the TLS destructors, ending up in initialized thread specific data never
483480
// being destroyed properly. Any other heap operation will do a full init.
484481
initThreadMaybe(/*MinimalInit=*/true);
485-
Options Options = Primary.Options.load();
486482

487483
#ifdef GWP_ASAN_HOOKS
488484
if (UNLIKELY(GuardedAlloc.pointerIsMine(Ptr))) {
@@ -491,7 +487,7 @@ class Allocator {
491487
}
492488
#endif // GWP_ASAN_HOOKS
493489

494-
if (&__scudo_deallocate_hook)
490+
if (UNLIKELY(&__scudo_deallocate_hook))
495491
__scudo_deallocate_hook(Ptr);
496492

497493
if (UNLIKELY(!Ptr))
@@ -506,11 +502,13 @@ class Allocator {
506502

507503
if (UNLIKELY(Header.State != Chunk::State::Allocated))
508504
reportInvalidChunkState(AllocatorAction::Deallocating, Ptr);
505+
506+
const Options Options = Primary.Options.load();
509507
if (Options.get(OptionBit::DeallocTypeMismatch)) {
510-
if (Header.OriginOrWasZeroed != Origin) {
508+
if (UNLIKELY(Header.OriginOrWasZeroed != Origin)) {
511509
// With the exception of memalign'd chunks, that can be still be free'd.
512-
if (UNLIKELY(Header.OriginOrWasZeroed != Chunk::Origin::Memalign ||
513-
Origin != Chunk::Origin::Malloc))
510+
if (Header.OriginOrWasZeroed != Chunk::Origin::Memalign ||
511+
Origin != Chunk::Origin::Malloc)
514512
reportDeallocTypeMismatch(AllocatorAction::Deallocating, Ptr,
515513
Header.OriginOrWasZeroed, Origin);
516514
}
@@ -527,8 +525,8 @@ class Allocator {
527525

528526
void *reallocate(void *OldPtr, uptr NewSize, uptr Alignment = MinAlignment) {
529527
initThreadMaybe();
530-
Options Options = Primary.Options.load();
531528

529+
const Options Options = Primary.Options.load();
532530
if (UNLIKELY(NewSize >= MaxAllowedMallocSize)) {
533531
if (Options.get(OptionBit::MayReturnNull))
534532
return nullptr;
@@ -611,8 +609,7 @@ class Allocator {
611609
// allow for potential further in-place realloc. The gains of such a trick
612610
// are currently unclear.
613611
void *NewPtr = allocate(NewSize, Chunk::Origin::Malloc, Alignment);
614-
if (NewPtr) {
615-
const uptr OldSize = getSize(OldPtr, &OldHeader);
612+
if (LIKELY(NewPtr)) {
616613
memcpy(NewPtr, OldTaggedPtr, Min(NewSize, OldSize));
617614
quarantineOrDeallocateChunk(Options, OldPtr, &OldHeader, OldSize);
618615
}
@@ -1057,10 +1054,9 @@ class Allocator {
10571054
}
10581055
// If the quarantine is disabled, the actual size of a chunk is 0 or larger
10591056
// than the maximum allowed, we return a chunk directly to the backend.
1060-
// Logical Or can be short-circuited, which introduces unnecessary
1061-
// conditional jumps, so use bitwise Or and let the compiler be clever.
1057+
// This purposefully underflows for Size == 0.
10621058
const bool BypassQuarantine =
1063-
!Quarantine.getCacheSize() | !Size | (Size > QuarantineMaxChunkSize);
1059+
!Quarantine.getCacheSize() || ((Size - 1) >= QuarantineMaxChunkSize);
10641060
if (BypassQuarantine) {
10651061
NewHeader.State = Chunk::State::Available;
10661062
Chunk::compareExchangeHeader(Cookie, Ptr, &NewHeader, Header);

0 commit comments

Comments
 (0)