dd-trace-php icon indicating copy to clipboard operation
dd-trace-php copied to clipboard

refactor(profiling): less unsafe code

Open morrisonlevi opened this issue 1 year ago • 2 comments

Description

This reduces unsafe:

  1. RUNTIME_PHP_VERSION_ID is atomic. The reads and writes are Relaxed because the only write happens in module init where no other threads exist.
  2. Use AtomicPtr for Profiler.system_settings, which removes the need for manually derived unsafe Sync and Send.

Motivation

I have been reviewing past work to remove the Mutex in the PROFILER global. The increased scrutiny lead me to change these things.

Reviewer checklist

  • [ ] Test coverage seems ok.
  • [ ] Appropriate labels assigned.

morrisonlevi avatar Jul 02 '24 20:07 morrisonlevi

Benchmarks

Benchmark execution time: 2024-07-02 20:53:03

Comparing candidate commit fe467837e35e5060ec5b88793c7ecd011b88b69d in PR branch levi/less-unsafe with baseline commit 44a07ba98de2062986a33cc1592a2b87533441a0 in branch master.

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

pr-commenter[bot] avatar Jul 02 '24 20:07 pr-commenter[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 79.52%. Comparing base (44a07ba) to head (fe46783).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2739      +/-   ##
============================================
+ Coverage     79.24%   79.52%   +0.28%     
  Complexity     2216     2216              
============================================
  Files           201      201              
  Lines         22595    22595              
============================================
+ Hits          17905    17969      +64     
+ Misses         4690     4626      -64     
Flag Coverage Δ
tracer-extension 78.82% <ø> (ø)
tracer-php 80.54% <ø> (+0.69%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

see 6 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 data Powered by Codecov. Last update 44a07ba...fe46783. Read the comment docs.

codecov-commenter avatar Jul 02 '24 20:07 codecov-commenter