dd-trace-php
dd-trace-php copied to clipboard
perf(profiling): use an arena-based string table
Description
This updates libdatadog to use a branch which has an arena-based string table. See Datadog/libdatadog#306.
The arena capacity probably needs some tweaking, and should probably be set to a specific target set by the team after careful consideration. The memory is lazily populated by the OS, at least, so it's not automatically used (asking for 512 MiB won't allocate all 512 MiB immediately, it will grow up to that limit and then stop).
Part of PROF-9123.
Overhead
Using the symfony/demo with PHP 8.3 using php-fpm, this netted me about 3-4% less total container memory. I expected there to be a difference because:
- There is less memory allocator fragmentation going on as strings are variable size.
- I'm packing the string size into a
u16, so we save a few bytes per string compared to ausize.
I'm happy with 3-4% off the container, and don't feel a need to try to specifically calculate the reduction of just libdatadog's memory.
I haven't attempted to measure CPU rigorously. Unscientifically, it looks on par with master. I would expect a very slight win with this PR because arena allocators are quick to allocate, and we can skip deallocation altogether (but deallocation is infrequent, roughly once per minute). Since eyeballing it looks on-par with master, which was the expectation, I won't dive into it deeper.
Reviewer checklist
- [ ] Test coverage seems ok.
- [ ] Appropriate labels assigned.
Benchmarks
Benchmark execution time: 2024-03-28 14:09:47
Comparing candidate commit b1d3e7cf4a89ba4aaf74961f654436427cd48d4d in PR branch levi/arena-string-table with baseline commit 8e98f4de9f6e50c5a59b1fee4fd14eb1f4c5c172 in branch master.
Found 0 performance improvements and 0 performance regressions! Performance is the same for 18 metrics, 3 unstable metrics.
Codecov Report
Merging #2511 (8d65bce) into master (2e1ed12) will increase coverage by
1.16%. Report is 4 commits behind head on master. The diff coverage isn/a.
:exclamation: Current head 8d65bce differs from pull request most recent head c1d7657. Consider uploading reports for the commit c1d7657 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## master #2511 +/- ##
============================================
+ Coverage 75.91% 77.08% +1.16%
Complexity 2574 2574
============================================
Files 240 214 -26
Lines 27029 23057 -3972
Branches 976 0 -976
============================================
- Hits 20519 17773 -2746
+ Misses 5990 5284 -706
+ Partials 520 0 -520
| Flag | Coverage Δ | |
|---|---|---|
| appsec-extension | ? |
|
| tracer-extension | 78.70% <ø> (ø) |
|
| tracer-php | 75.08% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
see 26 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 2e1ed12...c1d7657. Read the comment docs.
Superseded by #2668.