From 699ebb7bb560cfe36b7484d81df24fe5ce45af3a Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Thu, 22 Oct 2020 10:57:36 -0700 Subject: [PATCH 1/2] Improve Clone performance by 5-7x Before: ``` BenchmarkLogsClone-16 2022 526826 ns/op BenchmarkMetricsClone-16 92 11530154 ns/op BenchmarkTracesClone-16 237 4822875 ns/op ``` After: ``` BenchmarkLogsClone-16 17643 70484 ns/op BenchmarkMetricsClone-16 516 2226758 ns/op BenchmarkTracesClone-16 1286 853794 ns/op ``` Signed-off-by: Bogdan Drutu --- consumer/pdata/log.go | 12 +++--------- consumer/pdata/log_test.go | 18 ++++++++++++++++++ consumer/pdata/metric.go | 12 +++--------- consumer/pdata/metric_test.go | 18 ++++++++++++++++++ consumer/pdata/trace.go | 12 +++--------- consumer/pdata/trace_test.go | 18 ++++++++++++++++++ 6 files changed, 63 insertions(+), 27 deletions(-) diff --git a/consumer/pdata/log.go b/consumer/pdata/log.go index 2937f6050f8..ba0ff91833b 100644 --- a/consumer/pdata/log.go +++ b/consumer/pdata/log.go @@ -15,8 +15,6 @@ package pdata import ( - "github.com/gogo/protobuf/proto" - "go.opentelemetry.io/collector/internal" otlpcollectorlog "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/logs/v1" otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/logs/v1" @@ -78,13 +76,9 @@ func (ld Logs) FromOtlpProtoBytes(data []byte) error { // Clone returns a copy of Logs. func (ld Logs) Clone() Logs { - otlp := *ld.orig - resourceLogsClones := make([]*otlplogs.ResourceLogs, 0, len(otlp)) - for _, resourceLogs := range otlp { - resourceLogsClones = append(resourceLogsClones, - proto.Clone(resourceLogs).(*otlplogs.ResourceLogs)) - } - return Logs{orig: &resourceLogsClones} + rls := NewResourceLogsSlice() + ld.ResourceLogs().CopyTo(rls) + return Logs{orig: rls.orig} } // LogRecordCount calculates the total number of log records. diff --git a/consumer/pdata/log_test.go b/consumer/pdata/log_test.go index 6117688adc2..e2b79f2816f 100644 --- a/consumer/pdata/log_test.go +++ b/consumer/pdata/log_test.go @@ -87,3 +87,21 @@ func TestLogsFromInvalidOtlpProtoBytes(t *testing.T) { err := NewLogs().FromOtlpProtoBytes([]byte{0xFF}) assert.EqualError(t, err, "unexpected EOF") } + +func TestLogsClone(t *testing.T) { + logs := NewLogs() + fillTestResourceLogsSlice(logs.ResourceLogs()) + assert.EqualValues(t, logs, logs.Clone()) +} + +func BenchmarkLogsClone(b *testing.B) { + logs := NewLogs() + fillTestResourceLogsSlice(logs.ResourceLogs()) + b.ResetTimer() + for n := 0; n < b.N; n++ { + clone := logs.Clone() + if clone.ResourceLogs().Len() != logs.ResourceLogs().Len() { + b.Fail() + } + } +} diff --git a/consumer/pdata/metric.go b/consumer/pdata/metric.go index 82f2ff2678c..43a9b196e79 100644 --- a/consumer/pdata/metric.go +++ b/consumer/pdata/metric.go @@ -15,8 +15,6 @@ package pdata import ( - "github.com/gogo/protobuf/proto" - otlpcollectormetrics "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/metrics/v1" otlpmetrics "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/metrics/v1" ) @@ -83,13 +81,9 @@ func (md Metrics) FromOtlpProtoBytes(data []byte) error { // Clone returns a copy of MetricData. func (md Metrics) Clone() Metrics { - otlp := MetricsToOtlp(md) - resourceMetricsClones := make([]*otlpmetrics.ResourceMetrics, 0, len(otlp)) - for _, resourceMetrics := range otlp { - resourceMetricsClones = append(resourceMetricsClones, - proto.Clone(resourceMetrics).(*otlpmetrics.ResourceMetrics)) - } - return MetricsFromOtlp(resourceMetricsClones) + rms := NewResourceMetricsSlice() + md.ResourceMetrics().CopyTo(rms) + return Metrics{orig: rms.orig} } func (md Metrics) ResourceMetrics() ResourceMetricsSlice { diff --git a/consumer/pdata/metric_test.go b/consumer/pdata/metric_test.go index d99e79e67ea..740080895bb 100644 --- a/consumer/pdata/metric_test.go +++ b/consumer/pdata/metric_test.go @@ -692,6 +692,24 @@ func TestMetricsFromInvalidOtlpProtoBytes(t *testing.T) { assert.EqualError(t, err, "unexpected EOF") } +func TestMetricsClone(t *testing.T) { + metrics := NewMetrics() + fillTestResourceMetricsSlice(metrics.ResourceMetrics()) + assert.EqualValues(t, metrics, metrics.Clone()) +} + +func BenchmarkMetricsClone(b *testing.B) { + metrics := NewMetrics() + fillTestResourceMetricsSlice(metrics.ResourceMetrics()) + b.ResetTimer() + for n := 0; n < b.N; n++ { + clone := metrics.Clone() + if clone.ResourceMetrics().Len() != metrics.ResourceMetrics().Len() { + b.Fail() + } + } +} + func BenchmarkOtlpToFromInternal_PassThrough(b *testing.B) { resourceMetricsList := []*otlpmetrics.ResourceMetrics{ { diff --git a/consumer/pdata/trace.go b/consumer/pdata/trace.go index e5e0a56d146..605722601ee 100644 --- a/consumer/pdata/trace.go +++ b/consumer/pdata/trace.go @@ -15,8 +15,6 @@ package pdata import ( - "github.com/gogo/protobuf/proto" - otlpcollectortrace "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/trace/v1" otlptrace "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/trace/v1" ) @@ -70,13 +68,9 @@ func (td Traces) FromOtlpProtoBytes(data []byte) error { // Clone returns a copy of Traces. func (td Traces) Clone() Traces { - otlp := TracesToOtlp(td) - resourceSpansClones := make([]*otlptrace.ResourceSpans, 0, len(otlp)) - for _, resourceSpans := range otlp { - resourceSpansClones = append(resourceSpansClones, - proto.Clone(resourceSpans).(*otlptrace.ResourceSpans)) - } - return TracesFromOtlp(resourceSpansClones) + rss := NewResourceSpansSlice() + td.ResourceSpans().CopyTo(rss) + return Traces{orig: rss.orig} } // SpanCount calculates the total number of spans. diff --git a/consumer/pdata/trace_test.go b/consumer/pdata/trace_test.go index b86b1fc6f12..2eab71d553a 100644 --- a/consumer/pdata/trace_test.go +++ b/consumer/pdata/trace_test.go @@ -158,3 +158,21 @@ func TestTracesFromInvalidOtlpProtoBytes(t *testing.T) { err := NewTraces().FromOtlpProtoBytes([]byte{0xFF}) assert.EqualError(t, err, "unexpected EOF") } + +func TestTracesClone(t *testing.T) { + traces := NewTraces() + fillTestResourceSpansSlice(traces.ResourceSpans()) + assert.EqualValues(t, traces, traces.Clone()) +} + +func BenchmarkTracesClone(b *testing.B) { + traces := NewTraces() + fillTestResourceSpansSlice(traces.ResourceSpans()) + b.ResetTimer() + for n := 0; n < b.N; n++ { + clone := traces.Clone() + if clone.ResourceSpans().Len() != traces.ResourceSpans().Len() { + b.Fail() + } + } +} From 8be28d017302c60c7daa7568fd309c462e71c642 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Thu, 22 Oct 2020 11:25:21 -0700 Subject: [PATCH 2/2] Fix nil panic in Attributes copy Signed-off-by: Bogdan Drutu --- consumer/pdata/common.go | 6 +++++- consumer/pdata/log.go | 2 +- consumer/pdata/metric.go | 2 +- consumer/pdata/trace.go | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/consumer/pdata/common.go b/consumer/pdata/common.go index 799a92b602c..423fcac13a5 100644 --- a/consumer/pdata/common.go +++ b/consumer/pdata/common.go @@ -367,7 +367,11 @@ func (a AttributeValue) copyTo(dest *otlpcommon.AnyValue) { } func (a AttributeValue) CopyTo(dest AttributeValue) { - if *a.orig != nil && dest.IsNil() { + if *a.orig == nil { + *dest.orig = nil + return + } + if dest.IsNil() { dest.InitEmpty() } a.copyTo(*dest.orig) diff --git a/consumer/pdata/log.go b/consumer/pdata/log.go index ba0ff91833b..9d2e8996476 100644 --- a/consumer/pdata/log.go +++ b/consumer/pdata/log.go @@ -78,7 +78,7 @@ func (ld Logs) FromOtlpProtoBytes(data []byte) error { func (ld Logs) Clone() Logs { rls := NewResourceLogsSlice() ld.ResourceLogs().CopyTo(rls) - return Logs{orig: rls.orig} + return Logs(rls) } // LogRecordCount calculates the total number of log records. diff --git a/consumer/pdata/metric.go b/consumer/pdata/metric.go index 43a9b196e79..41e2b5d2f01 100644 --- a/consumer/pdata/metric.go +++ b/consumer/pdata/metric.go @@ -83,7 +83,7 @@ func (md Metrics) FromOtlpProtoBytes(data []byte) error { func (md Metrics) Clone() Metrics { rms := NewResourceMetricsSlice() md.ResourceMetrics().CopyTo(rms) - return Metrics{orig: rms.orig} + return Metrics(rms) } func (md Metrics) ResourceMetrics() ResourceMetricsSlice { diff --git a/consumer/pdata/trace.go b/consumer/pdata/trace.go index 605722601ee..572f4338165 100644 --- a/consumer/pdata/trace.go +++ b/consumer/pdata/trace.go @@ -70,7 +70,7 @@ func (td Traces) FromOtlpProtoBytes(data []byte) error { func (td Traces) Clone() Traces { rss := NewResourceSpansSlice() td.ResourceSpans().CopyTo(rss) - return Traces{orig: rss.orig} + return Traces(rss) } // SpanCount calculates the total number of spans.