Skip to content

Commit 56241b6

Browse files
committed
[scudo] Move the decision making of page release to hasChanceToReleasePages()
This change is only in SizeClassAllocator32. SizeClassAllocator64 has it implemented. Reviewed By: cferris Differential Revision: https://reviews.llvm.org/D158456
1 parent 7d15657 commit 56241b6

File tree

1 file changed

+85
-66
lines changed

1 file changed

+85
-66
lines changed

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

Lines changed: 85 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -877,59 +877,15 @@ template <typename Config> class SizeClassAllocator32 {
877877
if (UNLIKELY(BytesInFreeList == 0))
878878
return 0;
879879

880-
if (BytesInFreeList <= Sci->ReleaseInfo.BytesInFreeListAtLastCheckpoint)
881-
Sci->ReleaseInfo.BytesInFreeListAtLastCheckpoint = BytesInFreeList;
882-
883-
// Always update `BytesInFreeListAtLastCheckpoint` with the smallest value
884-
// so that we won't underestimate the releasable pages. For example, the
885-
// following is the region usage,
886-
//
887-
// BytesInFreeListAtLastCheckpoint AllocatedUser
888-
// v v
889-
// |--------------------------------------->
890-
// ^ ^
891-
// BytesInFreeList ReleaseThreshold
892-
//
893-
// In general, if we have collected enough bytes and the amount of free
894-
// bytes meets the ReleaseThreshold, we will try to do page release. If we
895-
// don't update `BytesInFreeListAtLastCheckpoint` when the current
896-
// `BytesInFreeList` is smaller, we may take longer time to wait for enough
897-
// freed blocks because we miss the bytes between
898-
// (BytesInFreeListAtLastCheckpoint - BytesInFreeList).
899-
const uptr PushedBytesDelta =
900-
BytesInFreeList - Sci->ReleaseInfo.BytesInFreeListAtLastCheckpoint;
901-
if (PushedBytesDelta < PageSize && ReleaseType != ReleaseToOS::ForceAll)
880+
// ====================================================================== //
881+
// Check if we have enough free blocks and if it's worth doing a page
882+
// release.
883+
// ====================================================================== //
884+
if (ReleaseType != ReleaseToOS::ForceAll &&
885+
!hasChanceToReleasePages(Sci, BlockSize, BytesInFreeList,
886+
ReleaseType)) {
902887
return 0;
903-
904-
const bool CheckDensity =
905-
isSmallBlock(BlockSize) && ReleaseType != ReleaseToOS::ForceAll;
906-
// Releasing smaller blocks is expensive, so we want to make sure that a
907-
// significant amount of bytes are free, and that there has been a good
908-
// amount of batches pushed to the freelist before attempting to release.
909-
if (CheckDensity && ReleaseType == ReleaseToOS::Normal)
910-
if (PushedBytesDelta < Sci->AllocatedUser / 16U)
911-
return 0;
912-
913-
if (ReleaseType == ReleaseToOS::Normal) {
914-
const s32 IntervalMs = atomic_load_relaxed(&ReleaseToOsIntervalMs);
915-
if (IntervalMs < 0)
916-
return 0;
917-
918-
// The constant 8 here is selected from profiling some apps and the number
919-
// of unreleased pages in the large size classes is around 16 pages or
920-
// more. Choose half of it as a heuristic and which also avoids page
921-
// release every time for every pushBlocks() attempt by large blocks.
922-
const bool ByPassReleaseInterval =
923-
isLargeBlock(BlockSize) && PushedBytesDelta > 8 * PageSize;
924-
if (!ByPassReleaseInterval) {
925-
if (Sci->ReleaseInfo.LastReleaseAtNs +
926-
static_cast<u64>(IntervalMs) * 1000000 >
927-
getMonotonicTimeFast()) {
928-
// Memory was returned recently.
929-
return 0;
930-
}
931-
}
932-
} // if (ReleaseType == ReleaseToOS::Normal)
888+
}
933889

934890
const uptr First = Sci->MinRegionIndex;
935891
const uptr Last = Sci->MaxRegionIndex;
@@ -966,21 +922,23 @@ template <typename Config> class SizeClassAllocator32 {
966922
BG.Batches.front()->getCount();
967923
const uptr BytesInBG = NumBlocks * BlockSize;
968924

969-
if (ReleaseType != ReleaseToOS::ForceAll &&
970-
BytesInBG <= BG.BytesInBGAtLastCheckpoint) {
971-
BG.BytesInBGAtLastCheckpoint = BytesInBG;
972-
continue;
973-
}
974-
const uptr PushedBytesDelta = BytesInBG - BG.BytesInBGAtLastCheckpoint;
975-
if (ReleaseType != ReleaseToOS::ForceAll && PushedBytesDelta < PageSize)
976-
continue;
925+
if (ReleaseType != ReleaseToOS::ForceAll) {
926+
if (BytesInBG <= BG.BytesInBGAtLastCheckpoint) {
927+
BG.BytesInBGAtLastCheckpoint = BytesInBG;
928+
continue;
929+
}
977930

978-
// Given the randomness property, we try to release the pages only if the
979-
// bytes used by free blocks exceed certain proportion of allocated
980-
// spaces.
981-
if (CheckDensity && (BytesInBG * 100U) / AllocatedGroupSize <
982-
(100U - 1U - BlockSize / 16U)) {
983-
continue;
931+
const uptr PushedBytesDelta = BytesInBG - BG.BytesInBGAtLastCheckpoint;
932+
if (PushedBytesDelta < PageSize)
933+
continue;
934+
935+
// Given the randomness property, we try to release the pages only if
936+
// the bytes used by free blocks exceed certain proportion of allocated
937+
// spaces.
938+
if (isSmallBlock(BlockSize) && (BytesInBG * 100U) / AllocatedGroupSize <
939+
(100U - 1U - BlockSize / 16U)) {
940+
continue;
941+
}
984942
}
985943

986944
// TODO: Consider updating this after page release if `ReleaseRecorder`
@@ -1035,6 +993,67 @@ template <typename Config> class SizeClassAllocator32 {
1035993
return TotalReleasedBytes;
1036994
}
1037995

996+
bool hasChanceToReleasePages(SizeClassInfo *Sci, uptr BlockSize,
997+
uptr BytesInFreeList, ReleaseToOS ReleaseType)
998+
REQUIRES(Sci->Mutex) {
999+
DCHECK_GE(Sci->FreeListInfo.PoppedBlocks, Sci->FreeListInfo.PushedBlocks);
1000+
const uptr PageSize = getPageSizeCached();
1001+
1002+
if (BytesInFreeList <= Sci->ReleaseInfo.BytesInFreeListAtLastCheckpoint)
1003+
Sci->ReleaseInfo.BytesInFreeListAtLastCheckpoint = BytesInFreeList;
1004+
1005+
// Always update `BytesInFreeListAtLastCheckpoint` with the smallest value
1006+
// so that we won't underestimate the releasable pages. For example, the
1007+
// following is the region usage,
1008+
//
1009+
// BytesInFreeListAtLastCheckpoint AllocatedUser
1010+
// v v
1011+
// |--------------------------------------->
1012+
// ^ ^
1013+
// BytesInFreeList ReleaseThreshold
1014+
//
1015+
// In general, if we have collected enough bytes and the amount of free
1016+
// bytes meets the ReleaseThreshold, we will try to do page release. If we
1017+
// don't update `BytesInFreeListAtLastCheckpoint` when the current
1018+
// `BytesInFreeList` is smaller, we may take longer time to wait for enough
1019+
// freed blocks because we miss the bytes between
1020+
// (BytesInFreeListAtLastCheckpoint - BytesInFreeList).
1021+
const uptr PushedBytesDelta =
1022+
BytesInFreeList - Sci->ReleaseInfo.BytesInFreeListAtLastCheckpoint;
1023+
if (PushedBytesDelta < PageSize)
1024+
return false;
1025+
1026+
// Releasing smaller blocks is expensive, so we want to make sure that a
1027+
// significant amount of bytes are free, and that there has been a good
1028+
// amount of batches pushed to the freelist before attempting to release.
1029+
if (isSmallBlock(BlockSize) && ReleaseType == ReleaseToOS::Normal)
1030+
if (PushedBytesDelta < Sci->AllocatedUser / 16U)
1031+
return false;
1032+
1033+
if (ReleaseType == ReleaseToOS::Normal) {
1034+
const s32 IntervalMs = atomic_load_relaxed(&ReleaseToOsIntervalMs);
1035+
if (IntervalMs < 0)
1036+
return false;
1037+
1038+
// The constant 8 here is selected from profiling some apps and the number
1039+
// of unreleased pages in the large size classes is around 16 pages or
1040+
// more. Choose half of it as a heuristic and which also avoids page
1041+
// release every time for every pushBlocks() attempt by large blocks.
1042+
const bool ByPassReleaseInterval =
1043+
isLargeBlock(BlockSize) && PushedBytesDelta > 8 * PageSize;
1044+
if (!ByPassReleaseInterval) {
1045+
if (Sci->ReleaseInfo.LastReleaseAtNs +
1046+
static_cast<u64>(IntervalMs) * 1000000 >
1047+
getMonotonicTimeFast()) {
1048+
// Memory was returned recently.
1049+
return false;
1050+
}
1051+
}
1052+
} // if (ReleaseType == ReleaseToOS::Normal)
1053+
1054+
return true;
1055+
}
1056+
10381057
SizeClassInfo SizeClassInfoArray[NumClasses] = {};
10391058

10401059
HybridMutex ByteMapMutex;

0 commit comments

Comments
 (0)