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

Tue Apr 19 12:51:26 PDT 2022

int3 added a subscriber: thakis.
int3 added a comment.

I think it's a bit unfortunate that the library-fication work here is making this change hard to land. @thakis and I brought up this exact concern in the original thread (https://discourse.llvm.org/t/rfc-revisiting-lld-as-a-library-design/58445/)...

It doesn't help that we don't have a good set of benchmarks across all 3 platforms & a CI system to run them. Differences in ad-hoc local measurements are hard to resolve.

I wonder if we could move forward with the `LLVM_THREAD_LOCAL` approach for now in order to unblock further work on parallelizing LLD. I'm sure we can put together enough parallelization wins in LLD-MachO to dwarf a 1% overhead, but that will probably take a few months. I don't know as much about the other LLD ports but I assume there are similar opportunities. It will be easier to eat a regression later after it has "paid for itself" with wins.

Also, if we really wanted to lock in the win from using thread-locals, I reckon we could use some clever macros to switch between `LLVM_THREAD_LOCAL` and `ThreadLocal` at compile time, depending on whether we are building a standalone or a library.

In summary, my asks are: 1. can we ship the `THREAD_LOCAL` version for now and fix it later? 2. In the meantime, could we start some discussions on a set of benchmarks to make future modifications of cross-platform LLD code easier to analyze?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122922