fix(prof): incompatibility with ext-grpc#3542
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3542 +/- ##
==========================================
- Coverage 61.78% 61.73% -0.05%
==========================================
Files 139 139
Lines 13051 13051
Branches 1712 1712
==========================================
- Hits 8063 8057 -6
- Misses 4225 4231 +6
Partials 763 763 see 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Benchmarks [ profiler ]Benchmark execution time: 2026-01-06 19:24:12 Comparing candidate commit 6d1be1f in PR branch Found 2 performance improvements and 2 performance regressions! Performance is the same for 25 metrics, 7 unstable metrics. scenario:php-profiler-exceptions-with-profiler
scenario:php-profiler-timeline-memory-with-profiler-and-timeline
|
realFlowControl
force-pushed
the
florian/fix-grpc
branch
from
aeec7e8 to
28bc41f
Compare
realFlowControl
changed the title
ext-grpcext-grpc
realFlowControl
force-pushed
the
florian/fix-grpc
branch
from
28bc41f to
6d1be1f
Compare
morrisonlevi
left a comment
morrisonlevi
left a comment
There was a problem hiding this comment.
Approved, would be awesome if you can make a test or even if we just need to do something similar to what grpc does, maybe our own badly behaved function gated by a function or something?
3 participants
Description
In
ext-grpcyou can register a plugin-callback for metadata.ext-grpcexecutes this callback function on another thread in theplugin_get_metadata()function. This is done even in NTS on a GRPC background thread which is not made a PHP thread. It only works, because the PHP main thread is blocked by a PHP function call, technically serving as a mutex for the engine.In #3175 we fixed
ext-grpccompatibility by following PHP's global model and not use thread locals in NTS builds, but it seems we introduced thread locals again in the mean time and as such we might crash in allocation profiling when collecting a sample and calculating the new sampling distance, whenext-grpcexecutes PHP code on a background thread.The basic problem is that this new thread is not made a PHP thread, so it never goes through
GINITand never initialises theALLOCATION_PROFILING_STATSwhich means all of the memory there is uninitialised.What this PR does to address this:
StdRnginstead ofThreadRngin PHP NTS (ThreadRnguses thread locals under the hood, we need to avoid this)ALLOCATION_PROFILING_STATSa globalstatic muton NTS builds to avoid having non-PHP threads read from un-init memorytls_allocation_profiling_stats.rstoprofiling_stats.rsYou can review this PR commit by commit.
An alternative to this PR could be to switch PHP's globals all together, but I believe that this step needs more exploration than just a simple PR. However, it could help to finally resolve this issue once and for all. In the meantime this PR fixes the compatibility issue with
ext-grpcwhile also removing a thread local access in PHP NTS versions in the allocator hot path 🎉Reviewer checklist