[PATCH] D140841: [DWARFLinkerNext] Add StringPool class.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

In D140841#4025120 <https://reviews.llvm.org/D140841#4025120>, @clayborg wrote:

> I think the code David is talking about is in ConstString.cpp.

thank you for pointing. somehow I missed it.

> LLDB uniques strings so that it can compare strings by comparing pointers in a unique string pool.

this patch(D140841 <https://reviews.llvm.org/D140841>) does the same.

> This string pool is thread safe and goes to great length to reduce any lock contention. You might be able to loot some code from there if needed.

Generally speaking two solutions are similar with following differences:

lldb pool uses array of single thread hash-maps while D140841 <https://reviews.llvm.org/D140841> uses single multi-thread hashmap.

lldb pool uses array of allocators while D140841 <https://reviews.llvm.org/D140841> uses thread local allocators.

Using multi-thread hashmap, in theory, might have the advantage of better performance in multi-thread mode.
I will compare numbers for both implementations and post here.

Also multi-thread hashmap from D132455 <https://reviews.llvm.org/D132455> is more general in that sense that it might be used for non strings.
f.e. dwarf Type pool could be implemented the same way. While solution based on StringMap might be used only for strings.

If thread locals allocators are not good then similar solution(array of allocators) might be used for D140841 <https://reviews.llvm.org/D140841>.
Then this solution would look pretty match as lldb pool.

> This approach seems to avoid that contention by creating thread local string pools? Those would then need to be merged later?

No, they should not be merged. Threads created by ThreadPool are live until destructor of ThreadPool is called. Thus all thread local data are available and might be referenced. StringPool contains pointers to the thread local data which should be valid until the destructor of ThreadPool is called.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140841