[PATCH] D54968: [ELF] --gdb-index: use const char *, uint32_t to replace CachedHashStringRef

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D54968#1310361 <https://reviews.llvm.org/D54968#1310361>, @ruiu wrote:

> I believe the memory savings of this patch is not free; it now has to scan the same string more than once to find a null terminator. Even though we want to reduce memory usage of lld for Google, in general, saving memory is not always the goal that we want to achieve. We need to get a right balance. To do that, can you tell me how much memory you can save with this patch, and how much is the performance penalty of this patch?


I think the memory saving may be negligible but interestingly my testing says it makes it faster when linking clang. My understanding is that the smaller size of `NameAttrs` improves caching a bit.

`cmake -H. -BStaticDebug -G Ninja -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXE_LINKER_FLAGS=-Wl,--gdb-index ...`
`rm bin/clang-8; ninja -C StaticDebug -v -n clang` to extract the clang command line, append '-###' to get the ld.lld command line. `bin/clang-8` is of 601,059,920 bytes

`perf stat -r 20 /tmp/link-clang` => run several times without and with the patch. Without the patch it is usually `4.884600870 seconds time elapsed` while with the patch, it is usually `4.743274463 seconds time elapsed`


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54968