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.
i.e. define it as:
// __p is not permitted to be a null pointer.
void
deallocate(pointer __p,
size_type __t __attribute__((__unused__)))
{
#if __cpp_sized_deallocation
// ...
#else
// ..
#endif
}
Or to reduce the duplication, maybe:
// __p is not permitted to be a null pointer.
void
deallocate(pointer __p, size_type __t)
{
#if __cpp_aligned_new
if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
{
::operator delete(__p,
#if __cpp_sized_deallocation
__t * sizeof(_Tp),
#endif
std::align_val_t(alignof(_Tp)));
return;
}
#endif
::operator delete(__p
#if __cpp_sized_deallocation
, __t * sizeof(_Tp)
#endif
);
}
Note jemalloc >= 5.1 is required to fix a bug with 0 sizes.
How serious is the bug? What are the symptoms?
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.
