[PATCH] D142318: [Support] Add PerThreadBumpPtrAllocator class.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 13 13:25:09 PDT 2023


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

I have a suggested comment and some nits, otherwise looks good. It'd be good to have @andrewng's review as well.



================
Comment at: llvm/include/llvm/Support/PerThreadBumpPtrAllocator.h:18
+
+/// PerThreadAllocator allows separating allocations by thread id.
+/// It is possible because ThreadPoolExecutor creates threads, keeps them until
----------------
Suggest:

PerThreadAllocator is used in conjunction with ThreadPoolExecutor to allow per-thread allocations. It wraps a possibly thread-unsafe allocator, e.g. BumpPtrAllocator.

PerThreadAllocator must be used inside ThreadPoolExecutor as it utilizes getThreadIndex, which is set by ThreadPoolExecutor threads.




================
Comment at: llvm/unittests/Support/PerThreadBumpPtrAllocatorTest.cpp:21
+  PerThreadBumpPtrAllocator Allocator;
+
+  uint64_t *Var =
----------------
delete blank line


================
Comment at: llvm/unittests/Support/PerThreadBumpPtrAllocatorTest.cpp:41
+
+  static size_t constexpr NumAllocations = 5000;
+
----------------
5000 seems too much. 1000 suffices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142318



More information about the llvm-commits mailing list