[scudo] Quarantine optimization
ClosedPublic

Authored by cryptoad on Wed, May 16, 10:16 AM.

Details

Summary

It turns out that the previous code construct was not optimizing the allocation
and deallocation of batches. The class id was read as a class member (even
though a precomputed one) and nothing else was optimized. By changing the
construct this way, the compiler actually optimizes most of the allocation and
deallocation away to only work with a single class id, which not only saves some
CPU but also some code footprint.

Diff Detail

cryptoad created this revision.Wed, May 16, 10:16 AM
Herald added subscribers: Restricted Project, delcypher. · View Herald TranscriptWed, May 16, 10:16 AM
  • const uptr BatchClassId = SizeClassMap::ClassID(sizeof(QuarantineBatch));

Shouldn't it be constexpr? Or at least static const?

  • const uptr BatchClassId = SizeClassMap::ClassID(sizeof(QuarantineBatch));

    Shouldn't it be constexpr? Or at least static const?

SizeClassMap::ClassID can't be made C++11 constexpr due to branches, so BatchClassId can't be made constexpr.
Making it static const makes the compiler use __cxa_guard_acquire & __cxa_guard_release which we don't want to be able to stay free of libc++ dependencies.

Making it static const makes the compiler use cxa_guard_acquire & cxa_guard_release which we don't want to be able to stay free of libc++ dependencies.

I meant keeping it as class member (not making it function-local static).

Making it static const makes the compiler use cxa_guard_acquire & cxa_guard_release which we don't want to be able to stay free of libc++ dependencies.

I meant keeping it as class member (not making it function-local static).

Sorry for the misunderstanding.
As a class member constexpr doesn't work for the previously mentioned reasons, and static const complains that the function SizeClassMap::ClassId is non-constexpr.

dvyukov accepted this revision.Wed, May 16, 10:59 AM

I see. Let's wait for C++14.

This revision is now accepted and ready to land.Wed, May 16, 10:59 AM
This revision was automatically updated to reflect the committed changes.