[PATCH] D133715: [ADT] Add HashMappedTrie

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D133715#4031396 <https://reviews.llvm.org/D133715#4031396>, @avl wrote:

>> But having a fast concurrent BumpPtrAllocator would be independently useful, and I'd suggest optimizing the allocator before bloating the default trie size.
>
> +1 for fast concurrent  ThreadSafeBumpPtrAllocator.
>
> What do you think about following alternative implementation?
>
>   class ThreadSafeBumpPtrAllocator {
>     ThreadSafeBumpPtrAllocator() {
>       size_t ThreadsNum = ThreadPoolStrategy.compute_thread_count();
>       allocators.resize(ThreadsNum);
>     }
>     
>     void* Allocate (size_t Num) {
>         size_t AllocatorIdx = getThreadIdx();
>         
>         return allocators[AllocatorIdx].Allocate(Num);
>     }
>   
>     std::vector<BumpPtrAllocator> allocators;
>   };
>   
>   static thread_local ThreadIdx;
>   
>   size_t getThreadIdx() {
>     return ThreadIdx;
>   }
>
> This implementation uses the fact that ThreadPoolExecutor creates a fixed number
> of threads(ThreadPoolStrategy.compute_thread_count()) and keeps them until destructed
> . ThreadPoolExecutor can initialise thread local field ThreadIdx to the proper thread index. 
> The getThreadIdx() could return index of thread inside ThreadPoolExecutor.Threads. 
> ThreadSafeBumpPtrAllocator keeps separate allocator for each thread. In this case each thread would
> always use separate allocator. No neccessary to have locks, cas operations, no races...

+1; seems worth experimenting with (downside is you have at least as many allocations as active threads, but maybe that’s fine); IIRC C++11 thread-local initialization is slow on Darwin so we might want `__thread` here, or maybe the `static` makes it fast; +1 to Steven’s comment this should move to the other review (also IMO this could be an incremental improvement that lands later).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133715