trace: use weak in TracerProvider for storing tracers

[yumosx]

#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

[yumosx]

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?

[dmathieu]

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).

[yumosx]

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.

[XSAM]

Suggested change

	namedTracer sync.Map
	namedTracer sync.Map // map[instrumentation.Scope]weak.Pointer[*tracer]

Annotate the expected type.

[XSAM]

I know this is coming from https://go.dev/blog/cleanups-and-weak. But using an infinite loop with an unclear break is a code smell.

[yumosx]

I'm not sure if this test method is correct or reasonable.

[yumosx]

If this tests are reasonable, weak+cleanup works better based on the results.

BenchmarkTracerCleanup/WithWeakAndCleanup-8         	  691965	      1630 ns/op	     592 B/op	      13 allocs/op
BenchmarkTracerCleanup/NoWeak,_NoCleanup
BenchmarkTracerCleanup/NoWeak,_NoCleanup-8          	  304090	     91877 ns/op	     253 B/op	       5 allocs/op

[pellared]

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).

[yumosx]

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.

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.