[PATCH] improve performance of std::allocator::deallocate

[Pádraig Brady]

On 02/26/2019 05:50 AM, Jonathan Wakely wrote:
> On 23/02/19 02:04 +0000, Pádraig Brady wrote:
>> Attached is a simple patch which has been extensively tested within
>> Facebook,
>> and is enabled by default in our code base.
>>
>> Passing the size to the allocator allows it to optimize deallocation,
>> and this was seen to significantly reduce the work required in jemalloc,
>> with about 40% reduction in CPU cycles in the free path.
>
> Thanks, the patch looks good.
>
> I would prefer to only change the function body, as otherwise LTO
> sometimes produces link-time warnings about ODR violations, because
> the same function is defined on two different lines. The ODR detection
> heuristics aren't smart enough to complain when the function
> declarator is always defined on the same line, but with two different
> function bodies.

We've not noticed issues here with LTO, though that could be
due to global enablement of -fsized-deallocation.
Anyway your suggestion is neater. Updated patch attached.
> Note jemalloc >= 5.1 is required to fix a bug with 0 sizes.
>
> How serious is the bug? What are the symptoms?
>
I've updated the commit summary to say it's a crash.
Arguably that's better than mem corruption.

> It looks like 5.1.0 is less than a year old, so older versions are
> presumably still in wide use.
>
> We could potentially workaround it by making
> new_allocator::allocate(0) call ::operator new(1) when
> __cpp_sized_deallocation is defined, and deallocate(p, 0) call
> ::operator delete(p, 1). Obviously I'd prefer not to do that, because
> the default operator new already has an equivalent check, and only
> programs linking to jemalloc require the workaround.
>
Right the jemalloc fix was released May 2018.
It would be great to avoid the extra workarounds.
Given this would be released about a year after the jemalloc fix was
released,
and that this would only manifest upon program rebuild,
I would be inclined to not add any workarounds.
For reference tcmalloc and glibc malloc were seen to work fine with this.

thanks!
Pádraig.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-std-allocator-deallocate-support-sized-deallocation.patch
Type: text/x-patch
Size: 1695 bytes
Desc: 0001-std-allocator-deallocate-support-sized-deallocation.patch
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20190227/e82dc573/attachment.bin>