perf: move EG() and CG() in ZTS builds into __thread storage - #23227
perf: move EG() and CG() in ZTS builds into __thread storage#23227henderkes wants to merge 12 commits into
Conversation
… packagers want to set the glibc tls surplus in their httpd package
…s are in TLS storage)
arnaud-lb
left a comment
arnaud-lb
left a comment
There was a problem hiding this comment.
I like the --with-tsrm-tls-model idea.
Did a first pass, but I will take the time to review carefully.
| jit->tls = ir_TLS( | ||
| tsrm_ls_cache_tcb_offset ? tsrm_ls_cache_tcb_offset : tsrm_tls_index, | ||
| tsrm_ls_cache_tcb_offset ? IR_NULL : tsrm_tls_offset); | ||
| jit->tls = ir_TLS(tsrm_tls_index, tsrm_tls_offset + offsetof(zend_tsrm_ls_cache, self)); |
There was a problem hiding this comment.
The JIT changes seem wrong.
Can we postpones these changes to a separate PR? I believe that ir_TLS() should just work, as *(void**)_tsrm_ls_cache is still valid due to the cache field in _zend_tsrm_ls_cache. We just need to change the zend_jit_get_tsrm_ls_cache() function?
In the separate PR we could change ir_TLS() so that it returns the address of _tsrm_ls_cache instead of loading it.
There was a problem hiding this comment.
Hmm, I can look into that later. I'm not sure here. Jumping on my PC and changing the other two remarks quickly, but don't have time for this right now.
There was a problem hiding this comment.
Edit: seems to work.
That said, the jit IR needs to be changed later anyway (I'll have to upstream this change), because the new global-dynamic branch is broken in IR today.
b78839c to
5d9e202
Compare
2 participants
replay of #22231
Moves EG and CG into __thread storage after all. We first moved them into constant offsets (#22287) from *_tsrm_ls_cache, but I couldn't find a way to stop gcc or clang from reloading _tsrm_ls_cache base pointer between function calls, leading to an extra pointer load once per function.
This eliminates the pointer load, making the access sequence to EG/CG just a single mov (x64) / mrs + add + ldr (aarch64) under local-exec. initial-exec likewise loses the pointer load so 3 -> 2 instructions (x64).
cc @arnaud-lb
What I'm adding here to counter the global-dynamic fallback slowdown is the option to explicitly opt-in to initial-exec, under the knowledge that host programs loading it will need to increase the static tls surplus. This is not an issue for package providers.
PS: Actually figured out that the explicit model choice doesn't happen for musl, not sure about IE (static tls surplus on musl?), but LE should work just fine. That's for another PR though.