Page MenuHomePhabricator

[scudo][standalone] Minor optimization & improvements
ClosedPublic

Authored by cryptoad on Nov 19 2019, 10:22 AM.

Details

Summary

A few small improvements and optimizations:

  • when refilling the free list, push back the last batch and return the front one: this allows to keep the allocations towards the front of the region;
  • instead of using 48 entries in the shuffle array, use a multiple of MaxNumCached;
  • make the maximum number of batches to create on refil a constant; ultimately it should be configurable, but that's for later;
  • initCache doesn't need to zero out the cache, it's already done.
  • it turns out that when using || or &&, the compiler is adamant on adding a short circuit for every part of the expression. Which ends up making somewhat annoying asm with lots of test and conditional jump. I am changing that to bitwise | or & in two place so that the generated code looks better. Added comments since it might feel weird to people.

This yields to some small performance gains overall, nothing drastic
though.

Diff Detail

Event Timeline

cryptoad created this revision.Nov 19 2019, 10:22 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 19 2019, 10:22 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

And I forgot in the message the part about only checking Size and not NeededSize, adding compiler check to make sure we do not overflow a uptr.
This allows us to skip a check, and doesn't impact our security.

hctim added inline comments.Nov 21 2019, 8:58 AM
compiler-rt/lib/scudo/standalone/combined.h
167

!!?

hctim accepted this revision.Nov 21 2019, 8:58 AM

Other than that, LGTM

This revision is now accepted and ready to land.Nov 21 2019, 8:58 AM
cryptoad added inline comments.Nov 21 2019, 9:09 AM
compiler-rt/lib/scudo/standalone/combined.h
167

Options.ZeroContents is technically not a boolean, and I really want to have a boolean when using a bitwise or (since the logical or adds a long condition jump).
Hence the double logical not.

hctim added inline comments.Nov 21 2019, 9:19 AM
compiler-rt/lib/scudo/standalone/combined.h
167

Maybe static_cast<bool>(Options.ZeroContents)? Looks more clear to me IMO, and compiles down to the same thing: https://godbolt.org/z/SrbU2p

cryptoad updated this revision to Diff 230472.Nov 21 2019, 9:41 AM
cryptoad marked 3 inline comments as done.

Replacing the !! construct with a static_cast<bool>.

This revision was automatically updated to reflect the committed changes.