Skip to content

perf: move EG() and CG() in ZTS builds into __thread storage - #23227

Open
henderkes wants to merge 29 commits into
php:masterfrom
henderkes:perf/1-tls-eg-cg
Open

perf: move EG() and CG() in ZTS builds into __thread storage#23227
henderkes wants to merge 29 commits into
php:masterfrom
henderkes:perf/1-tls-eg-cg

Conversation

@henderkes

Copy link
Copy Markdown
Contributor

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.

@arnaud-lb arnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the --with-tsrm-tls-model idea.

Did a first pass, but I will take the time to review carefully.

Comment thread ext/opcache/jit/zend_jit_ir.c Outdated
Comment thread TSRM/TSRM.h Outdated
Comment thread Zend/zend.c Outdated

@henderkes henderkes left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit of a pain to find these on mobile, so I'll pin just these two, but there's more.

It could eventually be reworked when all the symbols move directly into thread storage, but I've not even begun thinking about it. There's probably no point except for zend_ini_scanner_globals and what we still have in the front.

Comment thread TSRM/TSRM.h Outdated
#if defined(ZEND_WIN32) && !defined(LIBZEND_EXPORTS)
/* Windows can't dllexport __declspec(thread) symbols, so outside Zend each module
* keeps a per-module `void *` pointer and reaches EG/CG via the resource-id indirection. */
# define ZEND_TSRMLS_CACHE_T void *

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The symbol is referenced as just a void* on windows

Comment thread TSRM/TSRM.h
@henderkes

Copy link
Copy Markdown
Contributor Author

Created the aarch64 global-dynamic first first, but I may as well look into teaching the JIT to take the address of _tsrm_ls_cache + offset instead of the mandatory load for a bit of JIT speedup.

@henderkes

Copy link
Copy Markdown
Contributor Author

@arnaud-lb out of

zend_ini_scanner_globals))
virtual_cwd_globals))
zend_signal_globals_t))
zend_gc_globals_size())
php_core_globals))
sapi_globals_struct))
zend_accel_globals))
zend_jit_globals))

Which do you think would make sense moving too? gc globals probably and maybe virtual_cwd_globals?

@arnaud-lb

arnaud-lb commented Aug 20, 2026

Copy link
Copy Markdown
Member

alloc_globals is likely the most accessed global as every emalloc/efree and related fetch AG(mm_heap). I know that zend_alloc_globals is not exposed, but we could reserve enough space in _tsrm_ls_cache without exposing it:

struct _zend_tsrm_ls_cache {
	void *cache;
	void *self;
	zend_executor_globals eg;
	zend_compiler_globals cg;
	uint8_t ag[123];
	...
};

// zend_alloc.c:
ZEND_STATIC_ASSERT(sizeof(zend_tsrm_ls_cache.ag) >= sizeof(zend_alloc_globals));

#define AG(v) (((zend_alloc_globals*)&_tsrm_ls_cache.ag)->v)

@henderkes

henderkes commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

I think I already have a branch open for the AG move... perhaps it makes more sense moving it here, though.

Edit: my memory these days, it's already in. But I just realised we're keeping a useless write around.

Comment thread Zend/zend_gc.c
@henderkes

henderkes commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Created the aarch64 global-dynamic first first, but I may as well look into teaching the JIT to take the address of _tsrm_ls_cache + offset instead of the mandatory load for a bit of JIT speedup.

Proving too far outside my expertise. I think this is reviewable like this now, the JIT optimization could be done later in IR side to let us drop the self reference.

Unless @dstogov would like to have a go at it.

@arnaud-lb arnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good in general, but it's a bit hard to follow. Possibly grouping abstractions in TSRM would help? Right now you need to have TSRM.*, zend.c, zend_globals_macros.h to understand what's going on.

The Windows changes make sense, but maybe @shivammathur can take a look as well?

Comment thread Zend/zend_globals_macros.h Outdated
Comment thread Zend/zend_globals_macros.h Outdated
Comment thread Zend/zend_globals_macros.h Outdated
Comment thread Zend/zend_globals_macros.h Outdated
Comment thread Zend/zend.c Outdated
Comment thread win32/dllmain.c
Comment thread Zend/zend.c
ts_allocate_fast_id_at(&compiler_globals_id, &compiler_globals_offset, ZEND_CG_OFFSET, sizeof(zend_compiler_globals), (ts_allocate_ctor) compiler_globals_ctor, (ts_allocate_dtor) compiler_globals_dtor);
ts_allocate_fast_id_at(&executor_globals_id, &executor_globals_offset, ZEND_EG_OFFSET, sizeof(zend_executor_globals), (ts_allocate_ctor) executor_globals_ctor, (ts_allocate_dtor) executor_globals_dtor);
ts_allocate_tls_id(&compiler_globals_id, compiler_globals_tls_addr, sizeof(zend_compiler_globals), (ts_allocate_ctor) compiler_globals_ctor, (ts_allocate_dtor) compiler_globals_dtor);
ts_allocate_tls_id(&executor_globals_id, executor_globals_tls_addr, sizeof(zend_executor_globals), (ts_allocate_ctor) executor_globals_ctor, (ts_allocate_dtor) executor_globals_dtor);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLS-backed EG skips worker persistent-list cleanup. Please preserve that cleanup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether that will be possible to keep. Same problem as before when we moved AG and SCNG, we can't safely access another threads __thread variables.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a way to reach it safely. Adding a detach marker gets us cleanup if the thread hasn't exited yet, but if it has, there's nothing we can do.

The only way to achieve this is moving persistent_list out of __thread storage, meaning out of EG. That's a large (albeit mechanical) change, though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shivammathur I'm taking the reaction as the go-ahead for it. Going to bed now, but I should get to it on Sunday.

ts_free_resources(), ts_free_thread(), ts_free_id() and ts_apply_for_id()
matched entries with p->thread_id == tsrm_thread_id(). That is ambiguous
in exactly the case the surrounding code exists to handle: a stale entry
of a dead thread can carry the live thread's recycled id. Compare against
the entry that tsrm_tls_get() hands out instead.

The recycle path in ts_resource_ex() no longer runs the stale entry's
destructors. It used to point the TLS cache at an entry whose native TLS
block had died with its thread, so any destructor reaching for EG or CG
read freed memory. Leaking the dead thread's module globals is the better
trade; a child process that recycles thread ids gets respawned by the
SAPI anyway. Hold the shutdown marker across the window so that signal
handlers stay away from both the stale entry and its replacement.

Also reset tls_key, the TSRM tables and TSRMLS_CACHE on shutdown.
Comment thread TSRM/TSRM.c Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants