[libcxx-commits] [PATCH] D152208: [libc++] Introduce __make_uninitialized_buffer and use it instead of get_temporary_buffer

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed Jun 28 16:28:48 PDT 2023


philnik added a comment.

In D152208#4457550 <https://reviews.llvm.org/D152208#4457550>, @EricWF wrote:

> @philnik
>
> First, I appreciate the energy you're putting into this project. Your work doesn't go unnoticed.
>
> As for this std::stable_sort topic, I get your position.  Ideally, the performance difference between these 
> two approaches wouldn't be worth discussing. But reality often diverges from theory.
>
> The folks who raised concerns are experiencing substantial performance impacts.

Maybe I've missed it, but I didn't see any data other than "performance got worse".

> They've presented data to support this - the kind of proactive involvement we strive to cultivate in our community.
>
> However, some of the discourse seems a bit hasty. 
> Instead of outright dismissing these concerns, let's take a step back and understand the nuances.

My intention was never to dismiss the concern, but I'd like a specific use-case so I can understand what the problem is.

> If clarity is missing, ask for more details.
>
> It's also worth mentioning that referring to someone's  implementation as "bad" may not be the best choice of words. 
> A more understanding tone can make a difference in promoting a collaborative and respectful community.
>
> Your dedication to this project is appreciated. Let's keep improving it together.

Yeah, that might not have been the best choice of words.

In D152208#4457564 <https://reviews.llvm.org/D152208#4457564>, @davidxl wrote:

> Forgot to answer the common on the use of stable_sort:
>
> The stable sort is called by another common library not controlled directly by the user. Besides the behavior depends on the input. The problem will manifest when the array to be sorted is small.

Maybe the better approach would be to improve our `__stable_sort_switch` heuristic. Would it be possible for you to share the problematic use-case? I suspect you might be using a non-trivially-copy-assignable type which isn't that costly to copy, resulting in small allocations that don't make any sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152208



More information about the libcxx-commits mailing list