[PATCH] D133713: [Support] Introduce ThreadSafeAllocator

[Ben Langmuir via Phabricator via llvm-commits]

benlangmuir added inline comments.


================
Comment at: llvm/include/llvm/Support/ThreadSafeAllocator.h:19
+/// Thread-safe allocator adaptor. Uses an unfair lock on the assumption that
+/// contention here is extremely rare.
+///
----------------
"an unfair lock" should say "a spin lock".  You can have blocking unfair locks, it just means they don't provide guarantees against starvation under contention.  Similarly the paragraph below should not talk about fair/unfair since that's not the right distinction.


================
Comment at: llvm/include/llvm/Support/ThreadSafeAllocator.h:38
+public:
+  auto Allocate(size_t N = 1) {
+    LockGuard Lock(Flag);
----------------
I'm a bit skeptical of `= 1`. I think this comes from `SpecificBumpPtrAllocator`, where the memory is typed, but for e.g. `ThreadSafeAllocator<BumpPtrAllocator>` this will allocate a single byte.  I would just drop the default value, but if you want to keep it maybe do some sfinae magic to only apply on `SpecificBumpPtrAllocator`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133713