andygrove opened a new issue, #6213: URL: https://github.com/apache/datafusion-comet/issues/6213
### What is the problem the feature request solves? After #6166, the allocation accounting wrapper still costs about 2% on TPC-H SF100 Q21. `libcomet` is loaded with `dlopen` and is built as both a `cdylib` and an `rlib` (#6210), so its thread-locals use the general-dynamic TLS model, and every allocation and free makes one `__tls_get_addr` call to reach `LOCAL_DRIFT`. #6166 took that from two calls to one. Removing the last one needs a design without a thread-local. The initial-exec TLS model would avoid the call, but it is nightly-only (`-Z tls-model`), and it can make `dlopen` fail once glibc's surplus static TLS is used up. Q21 on a 32-core Linux box (Spark 4.1.1, 2 executors x 8 cores, glibc allocator), median of iterations 2 and 3 over 3 alternating runs of each build: | Build | Q21 median | vs no wrapper | | --- | --- | --- | | No wrapper | 30.21 s | | | Wrapper, before #6166 | 31.66 s | +4.8% | | Wrapper, #6166 | 30.85 s | +2.1% | `perf` over the same query, summed over both executors: | Build | `__rust_alloc` / `__rust_dealloc` self | `__tls_get_addr` | Total | | --- | --- | --- | --- | | No wrapper | 0.50% | 0.07% | 0.57% | | Wrapper, #6166 | 3.34% | 1.38% | 4.72% | Of the 4.2 points the wrapper adds, the lookup is 1.3. The other 2.8 are in `__rust_alloc` and `__rust_dealloc` themselves, which check the thread-local's state, update the drift, and account after the backend returns instead of tail-calling it. So the lookup bounds what dropping the thread-local can save, before paying for whatever replaces it. ### Describe the potential solution @comphead pointed to DuckDB in the [#6166 review](https://github.com/apache/datafusion-comet/pull/6166#pullrequestreview-5308339792). DuckDB is also often `dlopen`ed, and `BufferPool::MemoryUsage::UpdateUsedMemory` avoids thread-locals: it keeps 64 relaxed atomic counters indexed by `sched_getcpu()` and flushes each into the total at 32 KiB. That needs no thread-local and no settle at thread exit. The costs to measure against the 1.3 points: - `sched_getcpu` on every call. From glibc 2.35 it is a load from the thread's `rseq` area (checked in Ubuntu 22.04's glibc), and older glibc goes through the vDSO. - An atomic add on every call, where the current design does a plain add and touches the shared atomic about once per 64 KiB. A slot is normally touched by one core only, but a thread can migrate between `sched_getcpu` and the add, and CPUs share slots beyond 64. ### Additional context The `alloc_overhead` benchmark cannot see the lookup on Linux. It links `comet` as an `rlib` into an executable, where a thread-local access is a single instruction. On macOS it can, because Mach-O goes through the TLV thunk even in an executable, which is how #6166 measured it. A Linux A/B needs either a harness that `dlopen`s a `cdylib` built the way `libcomet` is (#6165 listed this under Validation) or the TPC-H Q21 A/B and `perf` profile from #6166. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
