perf: ZTS: move AG and SCNG into native __thread storage, replay of #22231

[henderkes]

So I largely copied the approach of #22231, but this time for AG and SCNG, because they're small in size (256 bytes, so we don't step back into the TLS surplus issue) and I don't need to touch the JIT.

EG and CG are "fine" to keep in the heap storage with the const offsets, because EG is mostly hot in dispatch and the __thread symbol is already kept loaded in the hybrid VM. For clang compile-heavy code still suffers a bit from tsrm_ls_cache base reloads, but I'm still brainstorming for that.

This mostly benefits the lexer and allocations.

@arnaud-lb

[arnaud-lb]

/* A __thread block of a foreign thread is inaccessible. */

Is that because the thread may not exist anymore, or is there another reason?

We need to ensure that shutdown_memory_manager(CG(unclean_shutdown), 1); is called in php_module_shutdown. Currently we avoid calling it in ZTS builds because we rely on the TSRM dtor, I believe.

I would go as far as removing the dtor argument from ts_allocate_tls_id since it's not going to be called for most threads during tsrm_shutdown().

[henderkes]

Is that because the thread may not exist anymore, or is there another reason?

It's pointing to __thread memory of another thread, but the only way it got here is if a foreign thread already died, so we'd be accessing dangling memory. It's technically accessible but there's no telling what running the dtor would lead to.

We need to ensure that shutdown_memory_manager(CG(unclean_shutdown), 1); is called in php_module_shutdown. Currently we avoid calling it in ZTS builds because we rely on the TSRM dtor, I believe.

Is this a general request? I don't change the main thread shutdown path here (it gets dtor defined and is guaranteed to run its own thread). If it's just a general request, I would postpone that to a follow-up PR.

I would go as far as removing the dtor argument from ts_allocate_tls_id since it's not going to be called for most threads during tsrm_shutdown().

Can't currently do that for AG because it does pass and run the dtor.

[arnaud-lb]

The dtor does run but only for the thread that calls tsrm_shutdown() and not for other threads. Before your changes, the dtor was called for all threads during tsrm_shutdown().

[henderkes]

The dtor does run but only for the thread that calls tsrm_shutdown() and not for other threads. Before your changes, the dtor was called for all threads during tsrm_shutdown().

The dtor does run for all threads that exit via ts_free_thread() -> ts_free_resources(). The clean shutdown logic is that no live threads will be registered here anymore except for the main thread during tsrm_shutdown, only failed/crashed threads (for which we can no-longer safely run the dtor for).

[arnaud-lb]

I now see that FrankenPHP calls ts_free_thread(), but other SAPIs don't: they rely on the current behavior of tsrm_shutdown() which is to cleanup all threads at once.

Let's keep the dtor argument since it's useful for SAPIs that call ts_free_thread().

Could you add a comment on tsrm_shutdown() and a note in UPGRADING.INTERNALS?

[henderkes]

Wait... what multi threaded sapi's don't call ts_free_thread()? Cli/fpm are single threaded and other webservers (pasir) or ext-parallel do call it.

Is this the apache handler again?

[henderkes]

It is, of course it is. I don't have a good solution for it, because it doesn't seem like Apache lets you install a thread shutdown hook that we could let ts_free_thread run on. We also can't safely reach into the threads memory without risking a segfault, so the memory just has to leak.

[arnaud-lb]

Yes this is Apache :) I think that's ok in Apache's context since its threads are only stopped when the process itself is going to be killed, which will release those resources.

[arnaud-lb]

Looks good to me otherwise!

[arnaud-lb]

Yes this is Apache :) I think that's ok in Apache's context since its threads are only stopped when the process itself is going to be killed, which will release those resources.

[arnaud-lb]

Thank you @henderkes!

[henderkes]

@arnaud-lb I'm kind of running out of ideas on how to prevent tls address reloads after every function call. Neither compiler gives the option and neither compiler is able to deem it safe to optimise. So as long as EG and CG stay in heap storage, pointed at by _tsrm_ls_cache + const offset it will always generate a two-instruction reload sequence after every function call. Less of an issue on gcc since all handlers get inlined the giant execute_ex(), but still not optimal.

I'm thinking about ignoring the tls surplus issue and just setting apache2handler's tls model to global-exec. It would obviously degrade apache2 handler quite a bit, but it would benefit cli, cgi, fpm and, notably what I care about, embed/frankenphp.

What do you think? Would this be acceptable? Perhaps cc @iliaal as an apache2handler maintainer.

[arnaud-lb]

Does it also happen with LTO? The compiler should be able to figure that _tsrm_ls_cache doesn't change when calling visible functions

An idea would be to declare _tsrm_ls_cache as a const pointer: __thread void * const _tsrm_ls_cache but updating its value is UB.

Otherwise, it would be interesting to benchmark the effect of global-exec on apache2handler. Maybe the other optimisations offset it?

[henderkes]

Clang 21 lto doesn't do it (it's beneficial, but doesn't omit tls reloads) and gcc can't compile with lto. I patched that to disable lto only on the zend_execute files (where the global register args break lto) but it didn't seem beneficial (though I didn't look at tls reloads).

An idea would be to declare _tsrm_ls_cache as a const pointer: __thread void * const _tsrm_ls_cache but updating its value is UB.

Hmm, that may be fine within core. I'll need to test it, but I can't imagine it works.

Otherwise, it would be interesting to benchmark the effect of global-exec on apache2handler. Maybe the other optimisations offset it?

I strongly doubt it... the globals are hot enough that the tls sequences going from 3 to 1 load account for most of the remaining difference. A function call which means register saving would be very costly, especially on Clang.

I'll benchmark it, if it's acceptable it would be preferable over keeping two entirely different tls layouts. One main motivation for all this is to get rid of NTS and reduce ZTS code complexity and ABI surface, keeping two different layouts in ZTS wouldn't work towards that goal.

[iliaal]

@henderkes Be interesting to see the apache2handler benchmark, I think Apache is still too big of a SAPI to regress in a big way even if other sapis are ok, but perhaps with a baseline there would be a meaningful starting point from which to optimize if necessary

[henderkes]

Gave it a quick spin and it regresses from +5% (compared to NTS, which it usually refuses to build as) to +10%. So ~4% regression.

Is the apache2handler actually still used? I thought fpm has been the recommended solution for apache for the last 10 years.

[iliaal]

TBH I've been using PHP with FPM on Nginx for the longest time, but if I did run Apache I'd probably turn to mod_php (apache2handler) since as I recall it was more performant than fpm in that environment, with a caveat it is rather date memory.

4% is not horrible, but probably worth tuning down to 1% if possible.

[henderkes]

Does it also happen with LTO? The compiler should be able to figure that _tsrm_ls_cache doesn't change when calling visible functions

Confirmed not working on either compiler with lto, tls reloads didn't change.

An idea would be to declare _tsrm_ls_cache as a const pointer: __thread void * const _tsrm_ls_cache but updating its value is UB.

That's segfaulting with nullptr reads, but aliasing it seems to work on clang. Makes me think perhaps a small asm accessor could be able to achieve this...

[henderkes]

Works, but not well enough and only on x86_64. I think the way forward is with moving all thread-native variables (globals) into native TLS. Looks to be around 20KB in total, so it'll of course be far past being able to load it when not linked against it in initial-exec TLS model. Windows wouldn't have an issue with it and neither would macOS I believe.

The question is really whether we need to keep the current heap layout as a fallback for the apache2handler SAPI or if we trade ~4% regression there for the simplification of not carrying two different tls layouts around.

TBH I've been using PHP with FPM on Nginx for the longest time, but if I did run Apache I'd probably turn to mod_php (apache2handler) since as I recall it was more performant than fpm in that environment, with a caveat it is rather date memory.

That would mean that FPM would have to be doing something terribly wrong, or you're in a network bound stack and don't have access to unix sockets. Since even the prefork MPM only builds with thread safety, it'll always be slower than NTS FPM or ZTS FPM with direct __thread storage. I sadly don't think the 4% cost would be tunable other than through a system setting opt-in (e.g. system packages could opt-in to initial-exec and get the performance uplift).