This is an archive of the discontinued LLVM Phabricator instance.

[scudo] Lazy initialize the PageMap while page releasing
ClosedPublic

Authored by Chia-hungDuan on Oct 27 2022, 11:51 AM.

Details

Summary

We allocate the page map before knowing if there're groups can be
released. This may result in many redundant map()/unmap() operations if
there's no page to release.

Make the page map be lazy initialized.

Diff Detail

Event Timeline

Chia-hungDuan created this revision.Oct 27 2022, 11:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2022, 11:51 AM
Herald added a subscriber: Enna1. · View Herald Transcript
Chia-hungDuan requested review of this revision.Oct 27 2022, 11:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2022, 11:51 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
cryptoad accepted this revision.Oct 27 2022, 2:41 PM
This revision is now accepted and ready to land.Oct 27 2022, 2:41 PM
cferris added inline comments.Oct 27 2022, 3:07 PM
compiler-rt/lib/scudo/standalone/release.h
269–270

Is it always the case that the NumberOfRegions, PagesCount, FullPagesBlockCountMax are the same when this is called? In other words, would you ever make this call and the values in the PageMap would need to be changed?

Chia-hungDuan added inline comments.Oct 27 2022, 3:21 PM
compiler-rt/lib/scudo/standalone/release.h
269–270

For each use in releaseToOSMaybe, those three variables will be the same. In different releaseToOSMaybe calls the PagesCount will change. It depends on the allocated size.

cferris accepted this revision.Oct 27 2022, 4:34 PM

LGTM

compiler-rt/lib/scudo/standalone/release.h
269–270

This looks good to me. I checked and the only place where PagesCount is modified, there is a reset call there so that this should never be out of sync.

This revision was landed with ongoing or failed builds.Oct 28 2022, 2:13 PM
This revision was automatically updated to reflect the committed changes.