[PATCH] D121635: [lld][macho][elf] Teach the bump-allocator in lld/Common about thread-safetiness.

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo created this revision.
oontvoo added reviewers: int3, MaskRay.
Herald added projects: lld-macho, All.
Herald added a reviewer: lld-macho.
oontvoo requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

The current impl is quite simple: hold a lock briefly while allocating new memory.
(Conceptually the bump-allocator can be implemented without locks using atomic incr', but looking at the current impl, I kind of got discouraged as it seemed too complex)
Performace data: (profiling chromium-framework):

x ./lld_macho_base
+ ./lld_macho_safe_alloc

SYSTEM CPU time:

  N           Min           Max        Median           Avg        Stddev

x   5          0.73          0.83          0.79          0.79   0.037416574
+   5          0.71          0.88          0.77         0.784   0.062689712
No difference proven at 95.0% confidence

USER CPU time:

  N           Min           Max        Median           Avg        Stddev

x   5          3.62          3.73          3.65          3.66   0.043588989
+   5          3.74          3.86          3.84         3.816   0.053665631
Difference at 95.0% confidence
	0.156 +/- 0.0712998
	4.2623% +/- 1.94808%
	(Student's t, pooled s = 0.0488876)

WALL time:

  N           Min           Max        Median           Avg        Stddev

x   5          4.56          4.61          4.59         4.588   0.019235384
+   5          4.68          4.74          4.72         4.714   0.024083189
Difference at 95.0% confidence
	0.126 +/- 0.031786
	2.74629% +/- 0.692808%
	(Student's t, pooled s = 0.0217945)

Use case:
LLD-macho occasionally allocate memory in multiple threads.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121635

Files:
  lld/Common/Memory.cpp
  lld/MachO/Driver.cpp
  lld/include/lld/Common/Memory.h


Index: lld/include/lld/Common/Memory.h
===================================================================
--- lld/include/lld/Common/Memory.h
+++ lld/include/lld/Common/Memory.h
@@ -22,8 +22,13 @@
 #define LLD_COMMON_MEMORY_H
 
 #include "llvm/Support/Allocator.h"
+#include <mutex>
 
 namespace lld {
+
+// Set to "true" for a thread-safe (but ~2.74% slower) allocator.
+extern bool ThreadSafeAlloc;
+
 // A base class only used by the CommonLinkerContext to keep track of the
 // SpecificAlloc<> instances.
 struct SpecificAllocBase {
@@ -39,6 +44,8 @@
   }
   llvm::SpecificBumpPtrAllocator<T> alloc;
   static int tag;
+
+  std::mutex mutex;
 };
 
 // The address of this static member is only used as a key in
@@ -48,18 +55,35 @@
 // Creates the arena on-demand on the first call; or returns it, if it was
 // already created.
 template <typename T>
-inline llvm::SpecificBumpPtrAllocator<T> &getSpecificAllocSingleton() {
+inline SpecificAlloc<T> *getSpecificAllocSingletonHelper() {
   SpecificAllocBase *instance = SpecificAllocBase::getOrCreate(
       &SpecificAlloc<T>::tag, sizeof(SpecificAlloc<T>),
       alignof(SpecificAlloc<T>), SpecificAlloc<T>::create);
-  return ((SpecificAlloc<T> *)instance)->alloc;
+  return (SpecificAlloc<T> *)instance;
+}
+
+template <typename T>
+inline llvm::SpecificBumpPtrAllocator<T> &getSpecificAllocSingleton() {
+  return getSpecificAllocSingletonHelper<T>()->alloc;
+}
+
+template <typename T> inline T *DoAlloc() {
+  {
+    auto *allocator = getSpecificAllocSingletonHelper<T>();
+    // TODO: maybe make the flag a compile-time config to avoid this branch.
+    if (ThreadSafeAlloc) {
+      std::lock_guard<std::mutex> lock(allocator->mutex);
+      return allocator->alloc.Allocate();
+    } else {
+      return allocator->alloc.Allocate();
+    }
+  }
 }
 
 // Creates new instances of T off a (almost) contiguous arena/object pool. The
 // instances are destroyed whenever lldMain() goes out of scope.
 template <typename T, typename... U> T *make(U &&... args) {
-  return new (getSpecificAllocSingleton<T>().Allocate())
-      T(std::forward<U>(args)...);
+  return new (DoAlloc<T>()) T(std::forward<U>(args)...);
 }
 
 } // namespace lld
Index: lld/MachO/Driver.cpp
===================================================================
--- lld/MachO/Driver.cpp
+++ lld/MachO/Driver.cpp
@@ -1107,6 +1107,7 @@
   if (errorCount())
     return false;
 
+  lld::ThreadSafeAlloc = true;
   if (args.hasArg(OPT_pagezero_size)) {
     uint64_t pagezeroSize = args::getHex(args, OPT_pagezero_size, 0);
 
Index: lld/Common/Memory.cpp
===================================================================
--- lld/Common/Memory.cpp
+++ lld/Common/Memory.cpp
@@ -12,6 +12,8 @@
 using namespace llvm;
 using namespace lld;
 
+bool lld::ThreadSafeAlloc = false;
+
 SpecificAllocBase *
 lld::SpecificAllocBase::getOrCreate(void *tag, size_t size, size_t align,
                                     SpecificAllocBase *(&creator)(void *)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D121635.415200.patch
Type: text/x-patch
Size: 2984 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220314/6734b2ba/attachment.bin>