census-instrumentation / opencensus-go

A stats collection and distributed tracing framework
http://opencensus.io
Apache License 2.0
2.06k stars 327 forks source link

Remove `convTslice` calls in Record() #1268

Closed howardjohn closed 3 years ago

howardjohn commented 3 years ago

This is built upon #1267; that one should likely merge first. I split this out as it has a small public API change (to work around circular imports) to avoid issues on the first PR.

Benchmark relative to #1267:

name                    old time/op    new time/op    delta
Record0-6                 1.77ns ± 1%    1.59ns ± 3%   -9.98%  (p=0.008 n=5+5)
Record1-6                  593ns ± 8%     544ns ± 8%   -8.40%  (p=0.095 n=5+5)
Record8-6                 1.20µs ± 3%    1.23µs ± 3%   +2.34%  (p=0.206 n=5+5)
Record8_WithRecorder-6     765ns ± 7%     783ns ± 6%     ~     (p=0.690 n=5+5)
Record8_Parallel-6        1.19µs ± 2%    1.19µs ± 6%     ~     (p=0.548 n=5+5)
Record8_8Tags-6           1.22µs ± 3%    1.22µs ± 3%     ~     (p=1.000 n=5+5)

name                    old alloc/op   new alloc/op   delta
Record0-6                  0.00B          0.00B          ~     (all equal)
Record1-6                   120B ± 0%       96B ± 0%  -20.00%  (p=0.008 n=5+5)
Record8-6                   344B ± 0%      320B ± 0%   -6.98%  (p=0.008 n=5+5)
Record8_WithRecorder-6      424B ± 0%      424B ± 0%     ~     (all equal)
Record8_Parallel-6          344B ± 0%      320B ± 0%   -6.98%  (p=0.008 n=5+5)
Record8_8Tags-6             344B ± 0%      320B ± 0%   -6.98%  (p=0.008 n=5+5)

name                    old allocs/op  new allocs/op  delta
Record0-6                   0.00           0.00          ~     (all equal)
Record1-6                   3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.008 n=5+5)
Record8-6                   3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.008 n=5+5)
Record8_WithRecorder-6      4.00 ± 0%      4.00 ± 0%     ~     (all equal)
Record8_Parallel-6          3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.008 n=5+5)
Record8_8Tags-6             3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.008 n=5+5)

For https://github.com/census-instrumentation/opencensus-go/issues/1265

howardjohn commented 3 years ago

Is this good to go?

dashpole commented 3 years ago

cc @punya for a second opinion