[scudo] Move the trace point in releaseToOSMaybe

[cferris1000]

Move the trace point until right before the mark free blocks call for the primary. This avoids adding extra calculations on a fast path.

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Christopher Ferris (cferris1000)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/159204.diff

2 Files Affected:

• (modified) compiler-rt/lib/scudo/standalone/primary32.h (+5-2)
• (modified) compiler-rt/lib/scudo/standalone/primary64.h (+5-2)

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<Config>::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<Config>::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<Config>::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<Config>::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<MemMapT> Recorder(&Region->MemMapInfo.MemMap,
                                           Region->RegionBeg,
                                           Context.getReleaseOffset());

[ChiaHungDuan]

markFreeBlocks is the most time consuming step, how about we add trace start from there?

[cferris1000]

Done.