pdatagen - Add MoveTo method for struct types #4240
Conversation
trasc
changed the title
|
The only difference is that you empty the src |
|
Completely wrong, this does not do a full copy, so definitely is a speed up. I have a concern here, the fact that we leave an "empty" message in the source slice. I think you want this to be used this in the ForEach so will not be a problem. |
For sure, in case the source is an element of a slice the caller should keep this in mind. It is specified in the methods comment but we cannot do much more then this. The same is happening in case of MoveAndAppendTo method generated for slices. |
|
@trasc where do you need this? I need to see few examples to convince myself about the usefulness of this. |
One use case is splitting a slice type (based on some attribute of its elements) for instance ResourceMetricsSlice. A benchmark for this can look like: func generateBenchmarksResourceMetricsSlice(l int) ResourceMetricsSlice {
ret := NewResourceMetricsSlice()
ret.EnsureCapacity(l)
for i := 0; i < l; i++ {
fillTestResourceMetrics(ret.AppendEmpty())
}
return ret
}
func BenchmarkMC_CopyTo(b *testing.B) {
src_rms := generateBenchmarksResourceMetricsSlice(b.N)
dst_rms := NewResourceMetricsSlice()
dst_rms.EnsureCapacity(b.N)
b.ResetTimer()
src_rms.RemoveIf(func(rm ResourceMetrics) bool {
if true { //some condition.. , but let's thake the worst case
rm.CopyTo(dst_rms.AppendEmpty())
return true
}
return false
})
}
func BenchmarkMC_MoveTo(b *testing.B) {
src_rms := generateBenchmarksResourceMetricsSlice(b.N)
dst_rms := NewResourceMetricsSlice()
dst_rms.EnsureCapacity(b.N)
b.ResetTimer()
src_rms.RemoveIf(func(rm ResourceMetrics) bool {
if true { //some condition.. , but let's thake the worst case
rm.MoveTo(dst_rms.AppendEmpty())
return true
}
return false
})
}and the results I'm getting: $ go test -run=none -bench=MC_ -benchtime=1000x -benchmem
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/collector/model/pdata
cpu: Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz
BenchmarkMC_CopyTo-12 1000 376079 ns/op 404112 B/op 6045 allocs/op
BenchmarkMC_MoveTo-12 1000 82.40 ns/op 80 B/op 1 allocs/op |
|
Please rebase. Also I think we should update the batch_processor to prove the usefulness of the change and add the benchmark results from there. |
- Fix metric split benchmark - Ignore clone counters in split benchmarks
|
Rebased and changed the batchprocessor to use MoveTo during splits. Benchmarks: Before: go test -run=none -bench=Split -benchtime=1000x -benchmem
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/collector/processor/batchprocessor
cpu: Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz
BenchmarkSplitLogs-12 1000 4339 ns/op 3200 B/op 53 allocs/op
BenchmarkSplitMetrics-12 1000 12308 ns/op 5056 B/op 109 allocs/op
BenchmarkSplitTraces-12 1000 4599 ns/op 4736 B/op 69 allocs/op
PASS
ok go.opentelemetry.io/collector/processor/batchprocessor 0.408sAfter: go test -run=none -bench=Split -benchtime=1000x -benchmem
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/collector/processor/batchprocessor
cpu: Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz
BenchmarkSplitLogs-12 1000 2719 ns/op 1968 B/op 27 allocs/op
BenchmarkSplitMetrics-12 1000 7505 ns/op 1456 B/op 27 allocs/op
BenchmarkSplitTraces-12 1000 2091 ns/op 2736 B/op 27 allocs/op
PASS
ok go.opentelemetry.io/collector/processor/batchprocessor 0.397s |
Codecov Report
@@ Coverage Diff @@
## main #4240 +/- ##
==========================================
+ Coverage 88.20% 88.38% +0.17%
==========================================
Files 173 173
Lines 10327 10434 +107
==========================================
+ Hits 9109 9222 +113
+ Misses 978 975 -3
+ Partials 240 237 -3
Continue to review full report at Codecov.
|
|
@trasc please ensure that we don't decrease our code coverage. |
|
@bogdandrutu can you trigger a re-run? |
Add a new move method in the generated pdata struct types. Changed the batchprocessor to use MoveTo during splits.
Benchmarks:
Before:
go test -run=none -bench=Split -benchtime=1000x -benchmem goos: linux goarch: amd64 pkg: go.opentelemetry.io/collector/processor/batchprocessor cpu: Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz BenchmarkSplitLogs-12 1000 4339 ns/op 3200 B/op 53 allocs/op BenchmarkSplitMetrics-12 1000 12308 ns/op 5056 B/op 109 allocs/op BenchmarkSplitTraces-12 1000 4599 ns/op 4736 B/op 69 allocs/op PASS ok go.opentelemetry.io/collector/processor/batchprocessor 0.408sAfter:
go test -run=none -bench=Split -benchtime=1000x -benchmem goos: linux goarch: amd64 pkg: go.opentelemetry.io/collector/processor/batchprocessor cpu: Intel(R) Core(TM) i7-9850H CPU @ 2.60GHz BenchmarkSplitLogs-12 1000 2719 ns/op 1968 B/op 27 allocs/op BenchmarkSplitMetrics-12 1000 7505 ns/op 1456 B/op 27 allocs/op BenchmarkSplitTraces-12 1000 2091 ns/op 2736 B/op 27 allocs/op PASS ok go.opentelemetry.io/collector/processor/batchprocessor 0.397s