On 02/26/2019 04:23 PM, Padraig Brady wrote:
>
>> 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.
Actually the jemalloc issue will only be fixed with the release of 5.2
(a few weeks away).
I've updated the commit message in the attached accordingly.
thanks,
Pádraig
From fec53f1500f5db6a2fe1b81c36a6695fd334758e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <pbr...@fb.com>
Date: Fri, 22 Feb 2019 17:21:57 -0800
Subject: [PATCH] std::allocator::deallocate support sized-deallocation
Pass the size to the allocator so that it may optimize deallocation.
This was seen to significantly reduce the work required in jemalloc,
with about 40% reduction in CPU cycles in the free path.
Note jemalloc >= 5.2 is required to fix a crash with 0 sizes.
* libstdc++-v3/include/ext/new_allocator.h (deallocate): Pass the size
to the deallocator with -fsized-deallocation.
---
libstdc++-v3/include/ext/new_allocator.h | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/libstdc++-v3/include/ext/new_allocator.h b/libstdc++-v3/include/ext/new_allocator.h
index e245391..f1ff7da 100644
--- a/libstdc++-v3/include/ext/new_allocator.h
+++ b/libstdc++-v3/include/ext/new_allocator.h
@@ -116,16 +116,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// __p is not permitted to be a null pointer.
void
- deallocate(pointer __p, size_type)
+ deallocate(pointer __p, size_type __t)
{
#if __cpp_aligned_new
if (alignof(_Tp) > __STDCPP_DEFAULT_NEW_ALIGNMENT__)
{
- ::operator delete(__p, std::align_val_t(alignof(_Tp)));
+ ::operator delete(__p,
+# if __cpp_sized_deallocation
+ __t * sizeof(_Tp),
+# endif
+ std::align_val_t(alignof(_Tp)));
return;
}
#endif
- ::operator delete(__p);
+ ::operator delete(__p
+#if __cpp_sized_deallocation
+ , __t * sizeof(_Tp)
+#endif
+ );
}
size_type
--
2.5.5
