From b68acb277f59f074a29df686cc0bc07fe30b737c Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Tue, 16 Sep 2025 15:08:23 -0700 Subject: [PATCH 1/2] [scudo] Move the trace point in releaseToOSMaybe Move the trace point until after it is determined that a release to os operation is needed in the releaseToOSMaybe function. This avoids adding extra calculations on a fast path. --- compiler-rt/lib/scudo/standalone/primary32.h | 7 +++++-- compiler-rt/lib/scudo/standalone/primary64.h | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/primary32.h b/compiler-rt/lib/scudo/standalone/primary32.h index 49aa74adfc10a..5cce34efb24d3 100644 --- a/compiler-rt/lib/scudo/standalone/primary32.h +++ b/compiler-rt/lib/scudo/standalone/primary32.h @@ -1058,8 +1058,6 @@ uptr SizeClassAllocator32::releaseToOSMaybe(SizeClassInfo *Sci, uptr ClassId, ReleaseToOS ReleaseType) REQUIRES(Sci->Mutex) { - SCUDO_SCOPED_TRACE(GetPrimaryReleaseToOSMaybeTraceName(ReleaseType)); - const uptr BlockSize = getSizeByClassId(ClassId); DCHECK_GE(Sci->FreeListInfo.PoppedBlocks, Sci->FreeListInfo.PushedBlocks); @@ -1104,6 +1102,11 @@ uptr SizeClassAllocator32::releaseToOSMaybe(SizeClassInfo *Sci, // ==================================================================== // // 3. Release the unused physical pages back to the OS. // ==================================================================== // + + // Only add trace point after it is determined that a release will occur to + // avoid incurring performance penalties. + SCUDO_SCOPED_TRACE(GetPrimaryReleaseToOSMaybeTraceName(ReleaseType)); + ReleaseRecorder Recorder(Base); auto SkipRegion = [this, First, ClassId](uptr RegionIndex) { ScopedLock L(ByteMapMutex); diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h index 7727049426b47..acc9b8c06ea0c 100644 --- a/compiler-rt/lib/scudo/standalone/primary64.h +++ b/compiler-rt/lib/scudo/standalone/primary64.h @@ -1379,8 +1379,6 @@ uptr SizeClassAllocator64::releaseToOSMaybe(RegionInfo *Region, uptr ClassId, ReleaseToOS ReleaseType) REQUIRES(Region->MMLock) EXCLUDES(Region->FLLock) { - SCUDO_SCOPED_TRACE(GetPrimaryReleaseToOSMaybeTraceName(ReleaseType)); - const uptr BlockSize = getSizeByClassId(ClassId); uptr BytesInFreeList; const uptr AllocatedUserEnd = @@ -1455,6 +1453,11 @@ uptr SizeClassAllocator64::releaseToOSMaybe(RegionInfo *Region, // ==================================================================== // // 4. Release the unused physical pages back to the OS. // ==================================================================== // + + // Only add trace point after it is determined that a release will occur to + // avoid incurring performance penalties. + SCUDO_SCOPED_TRACE(GetPrimaryReleaseToOSMaybeTraceName(ReleaseType)); + RegionReleaseRecorder Recorder(&Region->MemMapInfo.MemMap, Region->RegionBeg, Context.getReleaseOffset()); From 11362792cd1c0a5c4be1cfbf62ac3ccc23d9b67f Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Tue, 16 Sep 2025 17:28:39 -0700 Subject: [PATCH 2/2] Move the scope point to before the mark free blocks call. --- compiler-rt/lib/scudo/standalone/primary32.h | 11 ++++++----- compiler-rt/lib/scudo/standalone/primary64.h | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/primary32.h b/compiler-rt/lib/scudo/standalone/primary32.h index 5cce34efb24d3..645c3968ce3e1 100644 --- a/compiler-rt/lib/scudo/standalone/primary32.h +++ b/compiler-rt/lib/scudo/standalone/primary32.h @@ -1094,6 +1094,12 @@ uptr SizeClassAllocator32::releaseToOSMaybe(SizeClassInfo *Sci, // 2. Mark the free blocks and we can tell which pages are in-use by // querying `PageReleaseContext`. // ==================================================================== // + + // Only add trace point after the quick returns have occurred to avoid + // incurring performance penalties. Most of the time in this function + // will be the mark free blocks call and the actual release to OS call. + SCUDO_SCOPED_TRACE(GetPrimaryReleaseToOSMaybeTraceName(ReleaseType)); + PageReleaseContext Context = markFreeBlocks(Sci, ClassId, BlockSize, Base, NumberOfRegions, ReleaseType); if (!Context.hasBlockMarked()) @@ -1102,11 +1108,6 @@ uptr SizeClassAllocator32::releaseToOSMaybe(SizeClassInfo *Sci, // ==================================================================== // // 3. Release the unused physical pages back to the OS. // ==================================================================== // - - // Only add trace point after it is determined that a release will occur to - // avoid incurring performance penalties. - SCUDO_SCOPED_TRACE(GetPrimaryReleaseToOSMaybeTraceName(ReleaseType)); - ReleaseRecorder Recorder(Base); auto SkipRegion = [this, First, ClassId](uptr RegionIndex) { ScopedLock L(ByteMapMutex); diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h index acc9b8c06ea0c..d08103008ef7c 100644 --- a/compiler-rt/lib/scudo/standalone/primary64.h +++ b/compiler-rt/lib/scudo/standalone/primary64.h @@ -1442,6 +1442,12 @@ uptr SizeClassAllocator64::releaseToOSMaybe(RegionInfo *Region, // Then we can tell which pages are in-use by querying // `PageReleaseContext`. // ==================================================================== // + + // Only add trace point after the quick returns have occurred to avoid + // incurring performance penalties. Most of the time in this function + // will be the mark free blocks call and the actual release to OS call. + SCUDO_SCOPED_TRACE(GetPrimaryReleaseToOSMaybeTraceName(ReleaseType)); + PageReleaseContext Context = markFreeBlocks(Region, BlockSize, AllocatedUserEnd, getCompactPtrBaseByClassId(ClassId), GroupsToRelease); @@ -1453,11 +1459,6 @@ uptr SizeClassAllocator64::releaseToOSMaybe(RegionInfo *Region, // ==================================================================== // // 4. Release the unused physical pages back to the OS. // ==================================================================== // - - // Only add trace point after it is determined that a release will occur to - // avoid incurring performance penalties. - SCUDO_SCOPED_TRACE(GetPrimaryReleaseToOSMaybeTraceName(ReleaseType)); - RegionReleaseRecorder Recorder(&Region->MemMapInfo.MemMap, Region->RegionBeg, Context.getReleaseOffset());