opentelemetry-go
opentelemetry-go copied to clipboard
trace: use weak in TracerProvider for storing tracers
#6351
unique name
goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/sdk/trace
cpu: Apple M3
│ BenchmarkTracerWithUniqueName_output.txt │ afterUniqueName_output.txt │
│ sec/op │ sec/op vs base │
TracerWithUniqueName-8 389.9n ± 9% 918.1n ± 8% +135.44% (p=0.002 n=6)
│ BenchmarkTracerWithUniqueName_output.txt │ afterUniqueName_output.txt │
│ B/op │ B/op vs base │
TracerWithUniqueName-8 543.5 ± 15% 550.0 ± 0% ~ (p=1.000 n=6)
│ BenchmarkTracerWithUniqueName_output.txt │ afterUniqueName_output.txt │
│ allocs/op │ allocs/op vs base │
TracerWithUniqueName-8 4.000 ± 0% 11.000 ± 0% +175.00% (p=0.002 n=6)
same name:
goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/sdk/trace
cpu: Apple M3
│ BenchmarkTracerWithSameName_output.txt │ afterSameName_output.txt │
│ sec/op │ sec/op vs base │
TracerWithSameName-8 20.84n ± 1% 51.05n ± 1% +144.88% (p=0.002 n=6)
│ BenchmarkTracerWithSameName_output.txt │ afterSameName_output.txt │
│ B/op │ B/op vs base │
TracerWithSameName-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=6) ¹
¹ all samples are equal
│ BenchmarkTracerWithSameName_output.txt │ afterSameName_output.txt │
│ allocs/op │ allocs/op vs base │
TracerWithSameName-8 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=6) ¹
¹ all samples are equal
HI @pellared, I used weak to store the tracer (following the official Go blog's approach), but after performance testing, the results weren't good - performance actually decreased.
Am I using the wrong approach here?
The linked issue in the description seems unrelated to this PR.
Note that we won't be able to merge this until 1.25 is released (which is when we will drop support for 1.23).
The linked issue in the description seems unrelated to this PR.
Sorry, my bad, updated.
Note that we won't be able to merge this until 1.25 is released (which is when we will drop support for 1.23).
I know it, this pr is submitted as a PoC for early review.
as @pellared say
I hope we both learned something useful.
Hi, currently I have no time to review this PR, but I just want to call out that use of weak would not "improve" the performance (on the hot-path). It will only give the garbage collector the possibility to remove tracers that are not referenced by the user. So the total used memory should be lower (after garbage collection) when tracers are "used and forgotten" by the user (caller).
use of
weakwould not "improve" the performance (on the hot-path). It will only give the garbage collector the possibility to remove tracers that are not referenced by the user.
Yes, this is right, The initial Benchmark test show map[instrument.Scope]*tracer performs better than weak.Pointer[tracer].
So the total used memory should be lower (after garbage collection) when tracers are "used and forgotten" by the user (caller).
the second test, forcing runtime.GC() shows the weak+cleanup version runs faster than without it.
I think this improvement comes from weak+cleanup lowering GC pressure.
finally, thank you for the insightful reply.