Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.64%. Comparing base (373e64d) to head (eca063f).

@@            Coverage Diff             @@
##           master    #3505      +/-   ##
==========================================
- Coverage   61.75%   61.64%   -0.11%     
==========================================
  Files         143      143              
  Lines       13008    13008              
  Branches     1702     1702              
==========================================
- Hits         8033     8019      -14     
- Misses       4215     4228      +13     
- Partials      760      761       +1     

Benchmarks [ profiler ]

Benchmark execution time: 2025-12-09 16:06:11

Comparing candidate commit eca063f in PR branch florian/stabilize-and-speedup-allocations with baseline commit 373e64d in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 28 metrics, 7 unstable metrics.

scenario:php-profiler-exceptions-with-profiler-and-timeline

• 🟩 execution_time [-4.717ms; -2.514ms] or [-4.564%; -2.432%]

Benchmarks [ tracer ]

Benchmark execution time: 2025-12-08 08:31:57

Comparing candidate commit 476c702 in PR branch florian/stabilize-and-speedup-allocations with baseline commit 867b36c in branch master.

Found 3 performance improvements and 0 performance regressions! Performance is the same for 191 metrics, 0 unstable metrics.

scenario:ComposerTelemetryBench/benchTelemetryParsing

• 🟩 execution_time [-1138.261ns; -261.739ns] or [-10.073%; -2.316%]

scenario:MessagePackSerializationBench/benchMessagePackSerialization

• 🟩 execution_time [-5.456µs; -3.704µs] or [-4.892%; -3.321%]

scenario:TraceSerializationBench/benchSerializeTrace

• 🟩 execution_time [-46.715µs; -33.785µs] or [-10.212%; -7.386%]

With b0ce8e9 I've guarded the allocation (and exception) stats we are sending with the metadata JSON behind a feature flag because it looks like we are spending nearly ⅓ of our time just in incrementing these counters:

They are also only needed by us as a measurement to validate the upscaling algorithm. I am gonna measure again once the binary is ready to benchmark.

There is just no way that alloc_prof_realloc would call into alloc_prof_rshutdown and from there, there is no way to call into zend_mm_gc...

I don't think this quite right. The function zend_mm_safe_error is called by various allocation functions such as zend_mm_realloc_huge, and zend_mm_safe_error has zend_bailout which jumps to EG(bailout). It could jump to shutdown, yeah?

You are right, in case ZendMM can't allocate it bails out. Okay, that that part of the stack trace is fine. Just don't know how zend_mm_gc ends up there 😞

I was thinking: we might not need to do all of this get_or_init() dance and use a RefCell at runtime, when we can instead initialize the ALLOCATION_PROFILING_STATS in GINIT and then just "know" that this is initialized and use it.

RefCell is not about being initialized or not, that is the thread-local lazy initialization. The RefCell is about enforcing Rust's borrowing rules at runtime.

I think there's possibly some optimization though. We could use MaybeUninit<RefCell<AllocationProfilingStats>> and make it const { MaybeUninit::uninit() } so it doesn't need lazy-initialization checks at runtime. We initialize it in ginit. The RefCell still is there to enforce borrow rules at runtime (cover our butts, yes, it causes crashes, but that's better than undefined behavior).

We could benchmark the different options against each other and see. There are probably easier and more safe ways to bring down overhead without doing too much unsafe stuff 😉

The RefCell still is there to enforce borrow rules at runtime (cover our butts, yes, it causes crashes, but that's better than undefined behavior).

We are okay to use UnsafeCell instead of RefCell. We don't hold a reference while doing anything which could cause reentrancy. We need to better document this though.

I had a thought: when an Apache graceful restart is done, the process goes through mshutdown and back into minit. What is the story there with ginit and gshutdown?

I think this is my only concern before merging:

I had a thought: when an Apache graceful restart is done, the process goes through mshutdown and back into minit. What is the story there with ginit and gshutdown?

We need to be sure everything is okay. I am still getting back into the swing of work on a Monday morning, but I think it might be okay as-is, because the allocator hooks are unloaded so nothing should be accessing the un-initialized thread-locals even if gshutdown and such are called. If they aren't called, then they'll be okay from a safety perspective, but we'll have to think about correctness, perhaps?

I tried finding the code in the PHP extension that handles Apache graceful restarts and wasn't successful. Might be faster to spin up a new module with hooks and see when they are called under Apache, particularly if you already have a docker container that uses Apache set up (I don't currently).

I tried finding the code in the PHP extension that handles Apache graceful restarts and wasn't successful.

AFAIK this is handled in Apache itself, the entrypoint from Apache to PHP should be php_apache_server_startup().

Might be faster to spin up a new module with hooks and see when they are called under Apache, particularly if you already have a docker container that uses Apache set up (I don't currently).

I run the dd-trace-php version from the latest commit in this PR in a docker container with Apache and mpm_event in ZTS mode and did an apachectl -k graceful which worked flawless. The profiler shutdown and spun up again, no crashes 🎉