[compiler-rt] [scudo] Move the trace point in releaseToOSMaybe (PR #159204)

[Christopher Ferris via llvm-commits]

https://github.com/cferris1000 updated https://github.com/llvm/llvm-project/pull/159204

>From b68acb277f59f074a29df686cc0bc07fe30b737c Mon Sep 17 00:00:00 2001
From: Christopher Ferris <cferris at google.com>
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<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());

>From 11362792cd1c0a5c4be1cfbf62ac3ccc23d9b67f Mon Sep 17 00:00:00 2001
From: Christopher Ferris <cferris at google.com>
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<Config>::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<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 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<Config>::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<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());