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

Thu Jun 29 12:00:09 PDT 2023

EricWF added a comment.

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.

================
Comment at: libcxx/include/__memory/uninitialized_buffer.h:10
 #define _LIBCPP___MEMORY_UNINITIALIZED_BUFFER_H

----------------
ldionne wrote:
> Not attached to anything: would it be possible to upload the patch with context? This makes it easier to review. `arc diff` does that automatically, normally.
Yeah, my arc install is a bit broken - and  haven't had the time to fix it.

If you're looking to apply the patch, you can do so by downloading the raw patch, cd'ing into the `libcxx` directory, and running `git apply <raw-patch-file>`. 

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

https://reviews.llvm.org/D154017