[libcxx-commits] [PATCH] D154017: Cleanup __uninitialized_temporary_buffer internals.

[Fangrui Song via Phabricator via libcxx-commits]

MaskRay added a comment.

In D154017#4461107 <https://reviews.llvm.org/D154017#4461107>, @EricWF wrote:

> In D154017#4459618 <https://reviews.llvm.org/D154017#4459618>, @ldionne wrote:
>
>> @davidxl @mingmingl Did you folks notice the perf regression through libc++'s own benchmarks? If not, can you please share which benchmarks you used to see the difference? I'd like to add something to libc++'s benchmark suite alongside with this patch so we can have a concrete "regression test" and verify that this is making things better.
>>
>> Edit: We do have an ongoing problem in libc++ that we have some benchmarks and we run them on a regular basis in the CI, but we don't have any infrastructure that allows comparing benchmark results and detecting regressions. This makes these benchmarks a lot less useful than they could be.
>
> Yeah, benchmarking infrastructure is hard to set up, especially one that can largely go unsupervised. My understanding is that the benchmarks we have are more macroscale. I believe the benchmarks which caught this internally are closer to a macro-scale benchmark and  are dependent on a ton of infrastructure to gather and report macro-scale performance data, which makes it non-trivial to share externally.
>
> (Also it's an unfortunate truth of benchmarks that macroscale ones seem to be much better at detecting regressions than nice & tidy microbenchmarks).
>
> If somebody else knows more about the benchmarks that detected this, please chime in or correct me.

I have only read some mimalloc and jemalloc code and I suspect they are less affected by omitting ` if (__is_overaligned_for_new(__align))`.
(For mimalloc, it's almost not affected except a negligible overhead due to checking whether the alignment is a power of 2.)
tcmalloc seems to be affected more, but I have no insight into it.  `class_to_size`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154017/new/

https://reviews.llvm.org/D154017