This is an archive of the discontinued LLVM Phabricator instance.

[scudo][standalone] mallopt runtime configuration options
ClosedPublic

Authored by cryptoad on Jul 27 2020, 9:14 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)

Regarding 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.

This also fixes an issue where a static variable (Ready) was used in
templated functions without being set back to false every time.

Diff Detail

Event Timeline

cryptoad created this revision.Jul 27 2020, 9:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2020, 9:14 AM
Herald added subscribers: Restricted Project, jfb. · View Herald Transcript
cferris requested changes to this revision.Jul 27 2020, 10:47 AM

Is there any major changes from the other mallopt change? In other words, should I rerun performance data?

compiler-rt/lib/scudo/standalone/allocator_config.h
68

Super small nit, but I think this means default 1 TSD, max of 2 TSDs. So the comment should be max 2 TSDs.

compiler-rt/lib/scudo/standalone/combined.h
710

Is the compiler required to call setOptions if Result is false? Since the whole statement would be false, I think there is a chance a good enough compiler might not even make these calls if it notices that the first result or second result is false.

compiler-rt/lib/scudo/standalone/common.h
186

Small nit, should this align with the comments down below. Also, should MemtagTuning have a short explanation.

This revision now requires changes to proceed.Jul 27 2020, 10:47 AM
cryptoad updated this revision to Diff 280979.Jul 27 2020, 11:01 AM
cryptoad marked 3 inline comments as done.

Addressing Chrisopher's comments:

  • fixed a couple of comments
  • reworked setOption to avoid short circuiting

Is there any major changes from the other mallopt change? In other words, should I rerun performance data?

This is the same as the previous one, except for a couple of LIKELY/UNLIKELY in combined.h, which I can actually remove and save for a later CL.

cferris accepted this revision.Jul 27 2020, 12:33 PM

Okay, then this all looks good to me, and the previous perf results (that showed no perf difference) should carry over.

This revision is now accepted and ready to land.Jul 27 2020, 12:33 PM
This revision was landed with ongoing or failed builds.Jul 28 2020, 11:58 AM
This revision was automatically updated to reflect the committed changes.