Page MenuHomePhabricator

[scudo][standalone] mallopt runtime configuration options
AbandonedPublic

Authored by cryptoad on Jun 23 2020, 10:59 AM.

Details

Summary

Partners have requested the ability to configure more parts of Scudo
at runtime, notably the Secondary cache options (maximum number of
blocks cached, maximum size) as well as the TSD registry options
(the maximum number of TSDs in use).

This CL adds a few more Scudo specific mallopt parameters that are
passed down to the various subcomponents of the Combined allocator.

  • M_CACHE_COUNT_MAX: sets the maximum number of Secondary cached items
  • M_CACHE_SIZE_MAX: sets the maximum size of a cacheable item in the Secondary
  • M_TSDS_COUNT_MAX: sets the maximum number of TSDs that can be used (Shared Registry only)

Regardinbg the TSDs maximum count, this is a one way option, only
allowing to increase the count.

In order to allow for this, I rearranged the code to have some setOption
member function to the relevant classes, using the scudo::Option class
enum to determine what is to be set.

Diff Detail

Event Timeline

cryptoad created this revision.Jun 23 2020, 10:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 23 2020, 10:59 AM
Herald added subscribers: Restricted Project, jfb. · View Herald Transcript
cferris accepted this revision.Fri, Jul 17, 10:57 AM

I did a basic check that this doesn't cause any test failures on Android, nor does it change any RSS for the traces. I didn't do detailed perf testing since this shouldn't affect performance.

This revision is now accepted and ready to land.Fri, Jul 17, 10:57 AM
pcc added a comment.Thu, Jul 23, 4:44 PM

Where does the configuration need to live? If it just needs to be a build-time configuration, that may be simpler than exposing new mallopt options.

sungwook accepted this revision.Thu, Jul 23, 6:16 PM
sungwook added a subscriber: sungwook.

Verified code and it works fine.

In D82397#2171020, @pcc wrote:

Where does the configuration need to live? If it just needs to be a build-time configuration, that may be simpler than exposing new mallopt options.

It needs to be a per process decision that will be exercised at runtime (likely by apps).

pcc accepted this revision.Fri, Jul 24, 11:30 AM

A possible concern was that this could be constraining future development by exposing implementation details. But I think that as long as we are free to change how these options are interpreted, or start ignoring them entirely, this is probably fine.

This will probably require trivial merge conflict resolution after my change D84361.

cryptoad abandoned this revision.Mon, Jul 27, 9:16 AM

I created a new one @ https://reviews.llvm.org/D84667.
The content is the same but I had some issues rebasing/merging while keeping the same Phabricator revision, so I ended up redoing a new one.