Skip to content

Commit

Permalink
tracing: pass spanOptions by reference
Browse files Browse the repository at this point in the history
This reduces a bunch of struct copying. For now, I'm considering the
`none/parallel` benchmark diffs to be spurious.

```
❯ benchdiff  ./pkg/util/tracing -b -r 'BenchmarkTracer_StartSpanCtx' -d 100000x -c 10
test binaries already exist for 2b3e4f4: Merge #136225 #137136 #137216 #137275 #137281
checking out '9899e2d'
building benchmark binaries for 9899e2d: tracing: pass `spanOptions` by reference [bazel=true] 1/1 \

  pkg=1/1 iter=10/10 cockroachdb/cockroach/pkg/util/tracing \

name                                                                            old time/op    new time/op    delta
Tracer_StartSpanCtx/opts=none/parallel=false-10                                   50.0ns ± 6%    45.5ns ± 4%   -8.93%  (p=0.000 n=10+9)
Tracer_StartSpanCtx/opts=real,autoparent/parallel=false-10                         321ns ± 4%     301ns ± 1%   -6.36%  (p=0.000 n=10+8)
Tracer_StartSpanCtx/opts=real,manualparent/parallel=false-10                       343ns ± 4%     324ns ± 2%   -5.60%  (p=0.000 n=9+9)
Tracer_StartSpanCtx/opts=real,manualparent,withEventListener/parallel=false-10     358ns ± 2%     341ns ± 2%   -4.71%  (p=0.000 n=10+9)
Tracer_StartSpanCtx/opts=real,autoparent,withEventListener/parallel=false-10       472ns ± 2%     457ns ± 2%   -3.27%  (p=0.000 n=8+9)
Tracer_StartSpanCtx/opts=real/parallel=false-10                                    292ns ± 2%     282ns ± 2%   -3.15%  (p=0.000 n=8+9)
Tracer_StartSpanCtx/opts=real,manualparent,withEventListener/parallel=true-10      663ns ± 2%     643ns ± 4%   -3.11%  (p=0.002 n=9+9)
Tracer_StartSpanCtx/opts=real,logtag/parallel=false-10                             309ns ± 2%     300ns ± 2%   -3.02%  (p=0.000 n=9+9)
Tracer_StartSpanCtx/opts=real,autoparent/parallel=true-10                          621ns ± 3%     608ns ± 4%   -2.17%  (p=0.039 n=7+9)
Tracer_StartSpanCtx/opts=real,regexp/parallel=true-10                              826ns ± 2%     810ns ± 2%   -1.95%  (p=0.008 n=10+9)
Tracer_StartSpanCtx/opts=real/parallel=true-10                                     556ns ± 5%     561ns ± 3%     ~     (p=0.661 n=10+9)
Tracer_StartSpanCtx/opts=real,logtag/parallel=true-10                              580ns ± 2%     574ns ± 2%     ~     (p=0.077 n=9+9)
Tracer_StartSpanCtx/opts=real,manualparent/parallel=true-10                        635ns ± 3%     625ns ± 3%     ~     (p=0.074 n=8+9)
Tracer_StartSpanCtx/opts=real,autoparent,withEventListener/parallel=true-10       1.04µs ± 6%    1.03µs ± 5%     ~     (p=0.735 n=10+9)
Tracer_StartSpanCtx/opts=real,regexp/parallel=false-10                             745ns ± 1%     739ns ± 1%     ~     (p=0.078 n=8+8)
Tracer_StartSpanCtx/opts=none/parallel=true-10                                    15.3ns ±19%    22.1ns ±27%  +44.39%  (p=0.000 n=9+9)

name                                                                            old alloc/op   new alloc/op   delta
Tracer_StartSpanCtx/opts=none/parallel=false-10                                    48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=none/parallel=true-10                                     48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real/parallel=false-10                                    48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real/parallel=true-10                                     48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,logtag/parallel=false-10                             48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,logtag/parallel=true-10                              48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,autoparent/parallel=false-10                         48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,autoparent/parallel=true-10                          48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,manualparent/parallel=false-10                       48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,manualparent/parallel=true-10                        48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,autoparent,withEventListener/parallel=false-10       96.0B ± 0%     96.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,autoparent,withEventListener/parallel=true-10        96.0B ± 0%     96.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,manualparent,withEventListener/parallel=false-10     48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,manualparent,withEventListener/parallel=true-10      48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,regexp/parallel=false-10                             48.0B ± 0%     48.0B ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,regexp/parallel=true-10                              51.0B ± 0%     51.0B ± 0%     ~     (all equal)

name                                                                            old allocs/op  new allocs/op  delta
Tracer_StartSpanCtx/opts=none/parallel=false-10                                     1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=none/parallel=true-10                                      1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real/parallel=false-10                                     1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real/parallel=true-10                                      1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,logtag/parallel=false-10                              1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,logtag/parallel=true-10                               1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,autoparent/parallel=false-10                          1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,autoparent/parallel=true-10                           1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,manualparent/parallel=false-10                        1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,manualparent/parallel=true-10                         1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,autoparent,withEventListener/parallel=false-10        2.00 ± 0%      2.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,autoparent,withEventListener/parallel=true-10         2.00 ± 0%      2.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,manualparent,withEventListener/parallel=false-10      1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,manualparent,withEventListener/parallel=true-10       1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,regexp/parallel=false-10                              1.00 ± 0%      1.00 ± 0%     ~     (all equal)
Tracer_StartSpanCtx/opts=real,regexp/parallel=true-10                               1.00 ± 0%      1.00 ± 0%     ~     (all equal)

```
  • Loading branch information
dhartunian authored and yaothao committed Dec 18, 2024
1 parent 354d77a commit 039d0cd
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions pkg/util/tracing/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ func (t *Tracer) StartSpanCtx(
opts = os[i].apply(opts)
}

return t.startSpanFast(ctx, operationName, opts)
return t.startSpanFast(ctx, operationName, &opts)
}

// AlwaysTrace returns true if operations should be traced regardless of the
Expand All @@ -1098,8 +1098,12 @@ func (t *Tracer) forceOpNameVerbose(opName string) bool {
return false
}

// startSpanFast implements a fast path for the common case of tracing
// being disabled on the current span and its parent. We make only the
// checks necessary to ensure that recording is disabled and wrap the
// context in a `noopSpan`.
func (t *Tracer) startSpanFast(
ctx context.Context, opName string, opts spanOptions,
ctx context.Context, opName string, opts *spanOptions,
) (context.Context, *Span) {
if opts.RefType != childOfRef && opts.RefType != followsFromRef {
panic(errors.AssertionFailedf("unexpected RefType %v", opts.RefType))
Expand Down Expand Up @@ -1136,7 +1140,7 @@ func (t *Tracer) startSpanFast(
// the latter case, ctx == noCtx and the returned Context is the supplied one;
// otherwise the returned Context embeds the returned Span.
func (t *Tracer) startSpanGeneric(
ctx context.Context, opName string, opts spanOptions,
ctx context.Context, opName string, opts *spanOptions,
) (context.Context, *Span) {
if opts.RefType != childOfRef && opts.RefType != followsFromRef {
panic(errors.AssertionFailedf("unexpected RefType %v", opts.RefType))
Expand Down

0 comments on commit 039d0cd

Please sign in to comment.