andygrove commented on PR #6166: URL: https://github.com/apache/datafusion-comet/pull/6166#issuecomment-5824270908
@comphead I've run Q21 on this revision now, with all four builds re-run in one session on the same Linux box. The description has the full tables. | Build | Q21 median | vs no wrapper | | --- | --- | --- | | No wrapper | 30.21 s | | | Wrapper, `main` | 31.66 s | +4.8% | | Wrapper, first revision | 31.01 s | +2.6% | | Wrapper, this revision | 30.85 s | +2.1% | The two revisions are within each other's run-to-run spread, and `__tls_get_addr` is 1.38% of samples for both, against 2.60% on `main`. I also checked your x86_64 estimate against the release `libcomet.so`. The state check is the same in both revisions: a byte load, a compare and a taken branch. This revision saves one more callee-saved register than the first revision as pushed (r15, r14 and rbx, against r14 and rbx), because std's inlined `initialize` keeps the thread-local's address live across the destructor registration. In `dealloc` and `realloc` that register takes the place of the first revision's alignment push, so the instruction count doesn't change. `alloc` and `alloc_zeroed` get two more instructions, a `sub rsp` and a `pop`. So on x86_64 it costs one register rather than two push/pop pairs, and only those two methods pay for it. That fits about 0.2 points more `__rust_alloc` self time in the profile, and it doesn't show in Q21. It also refines what I said about the prologue earlier: against the first revision as pushed, the count only matches in `dealloc` and `realloc`. On the DuckDB idea, I've opened #6213. Of the 4.2 points the wrapper still adds, `__tls_get_addr` is 1.3 and the rest is in `__rust_alloc` and `__rust_dealloc` themselves. So 1.3 points is roughly what dropping the thread-local could save, before paying for `sched_getcpu` and an atomic add on every call. -- 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]
