[PATCH] D122922: [lld][common][lld-macho] Support per-thread allocators and StringSavers

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added inline comments.


================
Comment at: lld/Common/Memory.cpp:35
+PerThreadContext *CommonLinkerContext::perThreadContext() {
+  llvm::sys::RWMutex &contextMutex = context().contextMutex;
+  void *contextKey = &threadTag;
----------------
int3 wrote:
> I was hoping we could avoid mutexes altogether. I was thinking of something like
> 
> ```
> std::atomic<size_t> contextCount;
> std::array<PerThreadContext *, MAX_THREADS> perThreadContexts;
> 
> if (threadTag == 0) {
>   threadTag = contextCount++;
>   perThreadContexts[threadTag] = new PerThreadContext;
> }
> ```
What is the value of MAX_THREADS?  Doesn't that mean "max number of threads that can run concurrently" and not "max number of threads ever created during the application runtime"?

As new threads are spawned up, the arrays can expand and that'd be a potential race condition, yes?


================
Comment at: lld/include/lld/Common/CommonLinkerContext.h:24
+#include <thread>
+#include <unordered_map>
+
----------------
MaskRay wrote:
> Delete
> 
> unordered_map is very inefficient.
(removed - it was never used...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122922