Skip to content

Commit 85eb76f

Browse files
authored
Allow GC to collect unneeded slice elements (#5804)
```go type interInst struct { x int } type inter interface { } var sink []inter func BenchmarkX(b *testing.B) { sink = make([]inter, b.N) for i := 0; i < b.N; i++ { sink[i] = &interInst{} } clear(sink) sink = sink[:0] runtime.GC() var ms runtime.MemStats runtime.ReadMemStats(&ms) b.Log(b.N, ms.Frees) // Frees is the cumulative count of heap objects freed. } ``` ``` clear: ioz_test.go:35: 1 589 ioz_test.go:35: 100 711 ioz_test.go:35: 10000 10729 ioz_test.go:35: 1000000 1010750 <-- 1m+ freed ioz_test.go:35: 16076874 17087643 ioz_test.go:35: 19514749 36602412 ``` ``` no clear: ioz_test.go:35: 1 585 ioz_test.go:35: 100 606 ioz_test.go:35: 10000 725 ioz_test.go:35: 1000000 10745 <-- some "overhead" objects freed, not the slice. ioz_test.go:35: 16391445 1010765 ioz_test.go:35: 21765238 17402230 ``` This is documented at https://go.dev/wiki/SliceTricks: > NOTE If the type of the element is a pointer or a struct with pointer fields, which need to be garbage collected, the above implementations of Cut and Delete have a potential memory leak problem: some elements with values are still referenced by slice a’s underlying array, just not “visible” in the slice. Because the “deleted” value is referenced in the underlying array, the deleted value is still “reachable” during GC, even though the value cannot be referenced by your code. If the underlying array is long-lived, this represents a leak. Followed by examples of how zeroing out the slice elements solves the problem. This PR does the same.
1 parent 1492efa commit 85eb76f

File tree

7 files changed

+8
-5
lines changed

7 files changed

+8
-5
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
4646
- Support scope attributes and make them as identifying for `Meter` in `go.opentelemetry.io/otel` and `go.opentelemetry.io/otel/sdk/metric`. (#5926)
4747
- Support scope attributes and make them as identifying for `Logger` in `go.opentelemetry.io/otel` and `go.opentelemetry.io/otel/sdk/log`. (#5925)
4848
- Make schema URL and scope attributes as identifying for `Tracer` in `go.opentelemetry.io/otel/bridge/opentracing`. (#5931)
49+
- Clear unneeded slice elements to allow GC to collect the objects in `go.opentelemetry.io/otel/sdk/metric` and `go.opentelemetry.io/otel/sdk/trace`. (#5804)
4950

5051
### Removed
5152

sdk/log/record.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ func (r *Record) AddAttributes(attrs ...log.KeyValue) {
234234
//
235235
// Do not use head(attrs, r.attributeCountLimit - n) here. If
236236
// (r.attributeCountLimit - n) <= 0 attrs needs to be emptied.
237-
last := max(0, (r.attributeCountLimit - n))
237+
last := max(0, r.attributeCountLimit-n)
238238
r.addDropped(len(attrs) - last)
239239
attrs = attrs[:last]
240240
}

sdk/metric/internal/aggregate/drop.go

+1
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,6 @@ func (r *dropRes[N]) Offer(context.Context, N, []attribute.KeyValue) {}
2222

2323
// Collect resets dest. No exemplars will ever be returned.
2424
func (r *dropRes[N]) Collect(dest *[]exemplar.Exemplar) {
25+
clear(*dest) // Erase elements to let GC collect objects
2526
*dest = (*dest)[:0]
2627
}

sdk/metric/internal/aggregate/exemplar.go

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ var exemplarPool = sync.Pool{
1717
func collectExemplars[N int64 | float64](out *[]metricdata.Exemplar[N], f func(*[]exemplar.Exemplar)) {
1818
dest := exemplarPool.Get().(*[]exemplar.Exemplar)
1919
defer func() {
20+
clear(*dest) // Erase elements to let GC collect objects.
2021
*dest = (*dest)[:0]
2122
exemplarPool.Put(dest)
2223
}()

sdk/metric/pipeline.go

+2
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ func (p *pipeline) produce(ctx context.Context, rm *metricdata.ResourceMetrics)
132132
}
133133
if err := ctx.Err(); err != nil {
134134
rm.Resource = nil
135+
clear(rm.ScopeMetrics) // Erase elements to let GC collect objects.
135136
rm.ScopeMetrics = rm.ScopeMetrics[:0]
136137
return err
137138
}
@@ -145,6 +146,7 @@ func (p *pipeline) produce(ctx context.Context, rm *metricdata.ResourceMetrics)
145146
if err := ctx.Err(); err != nil {
146147
// This means the context expired before we finished running callbacks.
147148
rm.Resource = nil
149+
clear(rm.ScopeMetrics) // Erase elements to let GC collect objects.
148150
rm.ScopeMetrics = rm.ScopeMetrics[:0]
149151
return err
150152
}

sdk/trace/batch_span_processor.go

+1
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ func (bsp *batchSpanProcessor) exportSpans(ctx context.Context) error {
280280
//
281281
// It is up to the exporter to implement any type of retry logic if a batch is failing
282282
// to be exported, since it is specific to the protocol and backend being sent to.
283+
clear(bsp.batch) // Erase elements to let GC collect objects
283284
bsp.batch = bsp.batch[:0]
284285

285286
if err != nil {

sdk/trace/span.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -639,10 +639,7 @@ func (s *recordingSpan) dedupeAttrsFromRecord(record map[attribute.Key]int) {
639639
record[a.Key] = len(unique) - 1
640640
}
641641
}
642-
// s.attributes have element types of attribute.KeyValue. These types are
643-
// not pointers and they themselves do not contain pointer fields,
644-
// therefore the duplicate values do not need to be zeroed for them to be
645-
// garbage collected.
642+
clear(s.attributes[len(unique):]) // Erase unneeded elements to let GC collect objects.
646643
s.attributes = unique
647644
}
648645

0 commit comments

Comments
 (0)