Measurements.jl icon indicating copy to clipboard operation
Measurements.jl copied to clipboard

Use atomic tag counter

Open giordano opened this issue 2 years ago • 2 comments

I need think more about this, but this doesn't look good at all performance-wise. Before PR:

julia> @benchmark measurement(1, 2,)
BenchmarkTools.Trial: 10000 samples with 999 evaluations.
 Range (min … max):   7.706 ns …  1.856 μs  ┊ GC (min … max):  0.00% … 99.28%
 Time  (median):      9.483 ns              ┊ GC (median):     0.00%
 Time  (mean ± σ):   12.307 ns ± 43.490 ns  ┊ GC (mean ± σ):  10.97% ±  3.13%

  ▄▅▃▂██▅▃▂▁                           ▁ ▁▂▂▁▁                ▂
  ███████████▇▆▅▆▅▆▅▅▅▅▆▄▄▃▄▄▃▄▅▅▅▆▇▇██████████▆▆▄▆▆▅▆▆▆▆▆▆▆▅ █
  7.71 ns      Histogram: log(frequency) by time        28 ns <

 Memory estimate: 48 bytes, allocs estimate: 1.

After PR:

julia> @benchmark measurement(1, 2)
BenchmarkTools.Trial: 10000 samples with 192 evaluations.
 Range (min … max):  511.146 ns …  19.476 μs  ┊ GC (min … max): 0.00% … 96.25%
 Time  (median):     544.625 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   577.264 ns ± 528.867 ns  ┊ GC (mean ± σ):  2.86% ±  3.05%

  ▆▅▆▆▆█▅▃▂▂▂▃▂▂▂▂▃▃▁▁▁▂▁                                       ▂
  █████████████████████████▇▇█▇█▇▇▇▆▇▇▇▇█▇▇▆▅▆▆▆▇▆▄▃▄▅▆▅▆▅▅▆▄▁▅ █
  511 ns        Histogram: log(frequency) by time        908 ns <

 Memory estimate: 256 bytes, allocs estimate: 9.

I'm not really convinced it's worth killing performance this badly, the tag has a very minor role.

Also the benchmarks run in this PR confirm the bad effect on performance: https://github.com/JuliaPhysics/Measurements.jl/blob/3191a32b3043eb15d120de82371b2cfab33b077e/2022/03/28/214322/result.md#results

giordano avatar Mar 28 '22 21:03 giordano

Codecov Report

Merging #118 (ea061f8) into master (a5787bd) will decrease coverage by 0.01%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #118      +/-   ##
==========================================
- Coverage   95.57%   95.56%   -0.02%     
==========================================
  Files          13       13              
  Lines         746      744       -2     
==========================================
- Hits          713      711       -2     
  Misses         33       33              
Impacted Files Coverage Δ
src/Measurements.jl 87.09% <100.00%> (-0.79%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Apr 02 '22 17:04 codecov-commenter

With the atomic tag counter I get:

julia> @benchmark measurement(1, 2)
BenchmarkTools.Trial: 10000 samples with 999 evaluations.
 Range (min … max):  12.174 ns …  2.005 μs  ┊ GC (min … max): 0.00% … 98.94%
 Time  (median):     15.165 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   17.692 ns ± 48.266 ns  ┊ GC (mean ± σ):  8.53% ±  3.12%

  ▅▅▅▄▄▃▂█▇█▇▃▂  ▁                   ▁▁▂▂▃▂▁▂▁▁▁              ▂
  ██████████████▇█▇█▇▅▇▇▆▇▄▅▅▅▆▅▁▄▆▆▇████████████▇▅▆▆▆▇▇▇▇▇▇▇ █
  12.2 ns      Histogram: log(frequency) by time      32.6 ns <

 Memory estimate: 48 bytes, allocs estimate: 1.

Still slower than before, but the penalty is much more reasonable. Benchmarks: https://github.com/JuliaPhysics/Measurements.jl/blob/8b5405cd63498b2c9da5e4446eaea350bb1806b2/2022/04/02/174030/result.md#results

giordano avatar Apr 02 '22 17:04 giordano