Skip to content

[scudo] Add utilization percentages for stats. #75101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions compiler-rt/lib/scudo/standalone/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,21 @@ template <typename T> inline void shuffle(T *A, u32 N, u32 *RandState) {
*RandState = State;
}

inline void computePercentage(uptr Numerator, uptr Denominator, uptr *Integral,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking if we want a Pair in Scudo but I didn't get a good use case to add it. This form works for me too. Will leave it to you to make the decision.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was hoping I could use std::pair, but I think that would be a bad idea. And adding our own definition feels like a lot of work for a single case that's not necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree with this

uptr *Fractional) {
constexpr uptr Digits = 100;
if (Denominator == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we want to assert a non-zero denominator. It seems to give the impression that 0/1 and 0/0 are the same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another case I saw in real life, and I was wondering if I should say 100% when the denominator is 0. Since 0% feels like the wrong value, 100% also seems kind of wrong, but more correct than 0%. My other thought was doing NA, but that seems like it would be very difficult to implement without changing a lot of things.

I would think setting this to 100% seems the least bad and the most correct, but what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I think 100% is better! (Given that NA is more complicated)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set to 100, and modified the two locations that use it to reserve enough space for that 100%.

*Integral = 0;
*Fractional = 0;
return;
}

*Integral = Numerator * Digits / Denominator;
*Fractional =
(((Numerator * Digits) % Denominator) * Digits + Denominator / 2) /
Denominator;
}

// Platform specific functions.

extern uptr PageSizeCached;
Expand Down
8 changes: 6 additions & 2 deletions compiler-rt/lib/scudo/standalone/primary32.h
Original file line number Diff line number Diff line change
Expand Up @@ -931,10 +931,14 @@ template <typename Config> class SizeClassAllocator32 {
AllocatedPagesCount - Recorder.getReleasedPagesCount();
const uptr InUseBytes = InUsePages * PageSize;

uptr Integral;
uptr Fractional;
computePercentage(BlockSize * InUseBlocks, InUsePages * PageSize, &Integral,
&Fractional);
Str->append(" %02zu (%6zu): inuse/total blocks: %6zu/%6zu inuse/total "
"pages: %6zu/%6zu inuse bytes: %6zuK\n",
"pages: %6zu/%6zu inuse bytes: %6zuK util: %2zu.%02zu%%\n",
ClassId, BlockSize, InUseBlocks, TotalBlocks, InUsePages,
AllocatedPagesCount, InUseBytes >> 10);
AllocatedPagesCount, InUseBytes >> 10, Integral, Fractional);
}

NOINLINE uptr releaseToOSMaybe(SizeClassInfo *Sci, uptr ClassId,
Expand Down
8 changes: 6 additions & 2 deletions compiler-rt/lib/scudo/standalone/primary64.h
Original file line number Diff line number Diff line change
Expand Up @@ -1130,10 +1130,14 @@ template <typename Config> class SizeClassAllocator64 {
AllocatedPagesCount - Recorder.getReleasedPagesCount();
const uptr InUseBytes = InUsePages * PageSize;

uptr Integral;
uptr Fractional;
computePercentage(BlockSize * InUseBlocks, InUsePages * PageSize, &Integral,
&Fractional);
Str->append(" %02zu (%6zu): inuse/total blocks: %6zu/%6zu inuse/total "
"pages: %6zu/%6zu inuse bytes: %6zuK\n",
"pages: %6zu/%6zu inuse bytes: %6zuK util: %2zu.%02zu%%\n",
ClassId, BlockSize, InUseBlocks, TotalBlocks, InUsePages,
AllocatedPagesCount, InUseBytes >> 10);
AllocatedPagesCount, InUseBytes >> 10, Integral, Fractional);
}

NOINLINE uptr releaseToOSMaybe(RegionInfo *Region, uptr ClassId,
Expand Down
14 changes: 5 additions & 9 deletions compiler-rt/lib/scudo/standalone/secondary.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,20 +155,16 @@ template <typename Config> class MapAllocatorCache {

void getStats(ScopedString *Str) {
ScopedLock L(Mutex);
u32 Integral = 0;
u32 Fractional = 0;
if (CallsToRetrieve != 0) {
Integral = SuccessfulRetrieves * 100 / CallsToRetrieve;
Fractional = (((SuccessfulRetrieves * 100) % CallsToRetrieve) * 100 +
CallsToRetrieve / 2) /
CallsToRetrieve;
}
uptr Integral;
uptr Fractional;
computePercentage(SuccessfulRetrieves, CallsToRetrieve, &Integral,
&Fractional);
Str->append("Stats: MapAllocatorCache: EntriesCount: %d, "
"MaxEntriesCount: %u, MaxEntrySize: %zu\n",
EntriesCount, atomic_load_relaxed(&MaxEntriesCount),
atomic_load_relaxed(&MaxEntrySize));
Str->append("Stats: CacheRetrievalStats: SuccessRate: %u/%u "
"(%u.%02u%%)\n",
"(%zu.%02zu%%)\n",
SuccessfulRetrieves, CallsToRetrieve, Integral, Fractional);
for (CachedBlock Entry : Entries) {
if (!Entry.isValid())
Expand Down