[libcxx-commits] [PATCH] D128021: [libc++] Don't call key_eq in unordered_map/set rehashing routine

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D128021#3604428 <https://reviews.llvm.org/D128021#3604428>, @itrofimow wrote:

> In D128021#3603897 <https://reviews.llvm.org/D128021#3603897>, @Mordante wrote:
>
>> In D128021#3603708 <https://reviews.llvm.org/D128021#3603708>, @itrofimow wrote:
>>
>>> In D128021#3593922 <https://reviews.llvm.org/D128021#3593922>, @Mordante wrote:
>>>
>>>> 
>>>
>>> Would this example <https://pastebin.com/raw/64AgVneW> be enough?
>>> I could've messed up compile flags, and guidance would be appreciated if i did so
>>
>> Thanks! Can you provide the output of the `size` command instead of the `ls` command for both binaries?
>
> root at adelaide:~/tmp# size binary_size_*
>
>    text	   data	    bss	    dec	    hex	filename
>   10109	    744	      8	  10861	   2a6d	binary_size_main
>   11334	    744	      8	  12086	   2f36	binary_size_patch

Thanks. This gives more insight. For the ls the patched version is smaller, but here patched version is bigger; which matches my expectations. I'm not happy with the 1 KB increment of the text size. This price will be paid for every type.

I want to have a closer look at this myself and see whether there's a better solution that gives the performance improvement with a smaller text size overhead.



================
Comment at: libcxx/benchmarks/ContainerBenchmarks.h:140
+static void BM_Rehash(benchmark::State& st, Container c, GenInputs gen) {
+    auto in = gen(st.range(0));
+    c.max_load_factor(3.0);
----------------
Please don't use `auto` when the type isn't clear, same for the `bucket_count`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128021