[PATCH] D133713: [Support] Introduce ThreadSafeAllocator

[Steven Wu via Phabricator via llvm-commits]

steven_wu added a comment.

@dexonsmith Move the discussion here.

> Here's a sketch that I think mostly works?
>
>   // Block for bump allocations.
>   struct BumpBlock {
>     std::atomic<char *> Ptr;
>     std::atomic<BumpBlock *> Next;
>     char Bytes[4084];
>     BumpBlock() : Ptr{Bytes}, Next{nullptr} {}
>   
>     // Compute new "Next", try to bump if there's space, else return nullptr.
>     void *tryAllocate(size_t N, size_t Align = 1);
>   };
>   
>   // Tail-allocated data for "big" allocations.
>   struct BumpSeparate {
>     std::atomic<BumpSeparate *> Next;
>     // tail-allocated data of appropriate and alignment, using malloc....
>   };
>   
>   // Allocator.
>   struct BumpAllocator {
>     std::atomic<BumpBlock *> CurrentBlock;
>     std::atomic<BumpSeparate *> LastAllocSeparately;
>   
>     // Delete everything since there's no ownership here...
>     ~BumpAllocator();
>   
>     void *allocate(size_t N, size_t Align = 1) {
>       if (N > 2048)
>         return allocateSeparately(N, Align);
>   
>       BumpBlock *B = CurrentBlock;
>       std::unique_ptr<BumpBlock> New;
>       void *NewAlloc = nullptr;
>       while (true) {
>         if (LLVM_LIKELY(B))
>           if (void *Alloc = B->tryAllocate(N, Align))
>             return Alloc;
>   
>         if (!New) {
>           New = new BumpBlock;
>           NewAlloc = New->tryAllocate(N, Align);
>           assert(NewAlloc && "Empty block doesn't have space!");
>         }
>         if (!CurrentBlock.compare_exchange_weak(B, New.get()))
>           continue;
>   
>         // New was saved in CurrentBlock. Fix its "Next" pointer and release it
>         // so it's not deallocated.
>         New->Next = B;
>         New.release();
>         return NewAlloc;
>       }
>     }
>   
>   private:
>     // Maintain a list of "big" allocations, similar to above.
>     void *allocateSeparately(size_t N, size_t Align);
>   };
>
> Not saying it needs to block this review...
>
> But having a fast concurrent BumpPtrAllocator would be independently useful, and I'd suggest optimizing the allocator before bloating the default trie size.
>
> Yes, the FIXME for a ThreadSafeBumpPtrAllocator is still there. Currently, I don't think it is urgent to fix. It is not expected to have someone to use Trie as a high performance thread safe set/map.
> The immediate use case is as a high performance thread-safe data store. In the CAS use case we're expecting a lot of duplicate insertions which don't happen to hit the allocator, but I'm not sure that's really been checked/measured.

The algorithm works. Your sketch can have a small allocation in a slab when a conflict happens but that should be easy to fix.

The reason I don't want to have the CAS blocked by a better allocator is that I need to write and test it, also figure out if it needs to have any code sharing with regular BumpPtrAllocator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133713