From 59e5a08fd3185611e49e1ed2025f0e45fc376dcf Mon Sep 17 00:00:00 2001 From: toad Date: Fri, 7 Jun 2024 06:43:41 +0800 Subject: [PATCH 1/2] perf: Use non-generic to replace newEvictedQueue to reduce memory usage --- CHANGELOG.md | 1 + sdk/trace/evictedqueue.go | 17 +++++++++++------ sdk/trace/evictedqueue_test.go | 32 ++++++++++++++++---------------- sdk/trace/trace_test.go | 13 +++++++++++++ sdk/trace/tracer.go | 4 ++-- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14cd297b68f..9d9ef5566a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Changed - `Tracer.Start` in `go.opentelemetry.io/otel/trace/noop` no longer allocates a span for empty span context. (#5457) +- `Tracer.Start` in `go.opentelemetry.io/otel/sdk/trace` use non-generic to replace `newEvictedQueue` to reduce memory allocation. (#5497) ### Fixed diff --git a/sdk/trace/evictedqueue.go b/sdk/trace/evictedqueue.go index 3c62c3299c2..821c83faa1d 100644 --- a/sdk/trace/evictedqueue.go +++ b/sdk/trace/evictedqueue.go @@ -4,7 +4,6 @@ package trace // import "go.opentelemetry.io/otel/sdk/trace" import ( - "fmt" "slices" "sync" @@ -19,13 +18,19 @@ type evictedQueue[T any] struct { logDropped func() } -func newEvictedQueue[T any](capacity int) evictedQueue[T] { - var tVal T - msg := fmt.Sprintf("limit reached: dropping trace %T", tVal) +func newEvictedQueueEvent(capacity int) evictedQueue[Event] { // Do not pre-allocate queue, do this lazily. - return evictedQueue[T]{ + return evictedQueue[Event]{ capacity: capacity, - logDropped: sync.OnceFunc(func() { global.Warn(msg) }), + logDropped: sync.OnceFunc(func() { global.Warn("limit reached: dropping trace trace.Event") }), + } +} + +func newEvictedQueueLink(capacity int) evictedQueue[Link] { + // Do not pre-allocate queue, do this lazily. + return evictedQueue[Link]{ + capacity: capacity, + logDropped: sync.OnceFunc(func() { global.Warn("limit reached: dropping trace trace.Link") }), } } diff --git a/sdk/trace/evictedqueue_test.go b/sdk/trace/evictedqueue_test.go index 2a58ab345e2..7b88d63d077 100644 --- a/sdk/trace/evictedqueue_test.go +++ b/sdk/trace/evictedqueue_test.go @@ -14,47 +14,47 @@ func init() { } func TestAdd(t *testing.T) { - q := newEvictedQueue[string](3) - q.add("value1") - q.add("value2") + q := newEvictedQueueLink(3) + q.add(Link{}) + q.add(Link{}) if wantLen, gotLen := 2, len(q.queue); wantLen != gotLen { t.Errorf("got queue length %d want %d", gotLen, wantLen) } } func TestCopy(t *testing.T) { - q := newEvictedQueue[string](3) - q.add("value1") + q := newEvictedQueueEvent(3) + q.add(Event{Name: "value1"}) cp := q.copy() - q.add("value2") - assert.Equal(t, []string{"value1"}, cp, "queue update modified copy") + q.add(Event{Name: "value2"}) + assert.Equal(t, []Event{{Name: "value1"}}, cp, "queue update modified copy") - cp[0] = "value0" - assert.Equal(t, "value1", q.queue[0], "copy update modified queue") + cp[0] = Event{Name: "value0"} + assert.Equal(t, Event{Name: "value1"}, q.queue[0], "copy update modified queue") } func TestDropCount(t *testing.T) { - q := newEvictedQueue[string](3) + q := newEvictedQueueEvent(3) var called bool q.logDropped = func() { called = true } - q.add("value1") + q.add(Event{Name: "value1"}) assert.False(t, called, `"value1" logged as dropped`) - q.add("value2") + q.add(Event{Name: "value2"}) assert.False(t, called, `"value2" logged as dropped`) - q.add("value3") + q.add(Event{Name: "value3"}) assert.False(t, called, `"value3" logged as dropped`) - q.add("value1") + q.add(Event{Name: "value1"}) assert.True(t, called, `"value2" not logged as dropped`) - q.add("value4") + q.add(Event{Name: "value4"}) if wantLen, gotLen := 3, len(q.queue); wantLen != gotLen { t.Errorf("got queue length %d want %d", gotLen, wantLen) } if wantDropCount, gotDropCount := 2, q.droppedCount; wantDropCount != gotDropCount { t.Errorf("got drop count %d want %d", gotDropCount, wantDropCount) } - wantArr := []string{"value3", "value1", "value4"} + wantArr := []Event{{Name: "value3"}, {Name: "value1"}, {Name: "value4"}} gotArr := q.copy() if wantLen, gotLen := len(wantArr), len(gotArr); gotLen != wantLen { diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index 435c88849f6..bca58219a5e 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -2107,3 +2107,16 @@ func TestAddLinkToNonRecordingSpan(t *testing.T) { t.Errorf("AddLinkToNonRecordingSpan: -got +want %s", diff) } } + +func BenchmarkTraceStart(b *testing.B) { + tracer := NewTracerProvider().Tracer("") + ctx := trace.ContextWithSpanContext(context.Background(), trace.SpanContext{}) + + b.ReportAllocs() + b.ResetTimer() + + for i := 0; i < b.N; i++ { + _, span := tracer.Start(ctx, "") + span.End() + } +} diff --git a/sdk/trace/tracer.go b/sdk/trace/tracer.go index 8c4142d6f29..43419d3b541 100644 --- a/sdk/trace/tracer.go +++ b/sdk/trace/tracer.go @@ -132,8 +132,8 @@ func (tr *tracer) newRecordingSpan(psc, sc trace.SpanContext, name string, sr Sa spanKind: trace.ValidateSpanKind(config.SpanKind()), name: name, startTime: startTime, - events: newEvictedQueue[Event](tr.provider.spanLimits.EventCountLimit), - links: newEvictedQueue[Link](tr.provider.spanLimits.LinkCountLimit), + events: newEvictedQueueEvent(tr.provider.spanLimits.EventCountLimit), + links: newEvictedQueueLink(tr.provider.spanLimits.LinkCountLimit), tracer: tr, } From 823793499d0f0258d4f3312537bf2199881a870e Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 17 Jun 2024 07:24:00 -0700 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8f4b7e5cc4..77728de1b85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The exporter no longer exports the deprecated "otel.library.name" or "otel.library.version" attributes. - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/sdk/resource`. (#5490) - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/sdk/trace`. (#5490) -- `Tracer.Start` in `go.opentelemetry.io/otel/sdk/trace` use non-generic to replace `newEvictedQueue` to reduce memory allocation. (#5497) +- Use non-generic functions in the `Start` method of `"go.opentelemetry.io/otel/sdk/trace".Trace` to reduce memory allocation. (#5497) ### Fixed