Add pdata benchmarks to charts #73
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are interesting because they represent better what happens when STEF is used in Otel Collector.
There was a problem hiding this comment.
Pull Request Overview
This PR adds pdata-related benchmarks and integrates them into the charts, providing a clearer view of performance when using STEF in the Otel Collector.
- Updated chart container IDs and options in benchmarks/results/benchmarks.html
- Modified the benchmarks_test.go to use chart hooks (BeginChart, Record, EndChart) for pdata benchmarks
- Updated the makefile to include the new Pdata benchmark and changed STEF encoding naming in benchmarks/encodings/stef/stef.go
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| benchmarks/results/benchmarks.html | Updated chart container IDs and adjusted xAxis labels for sorted charts |
| benchmarks/makefile | Modified regex to run new Pdata benchmarks |
| benchmarks/encodings/stef/stef.go | Changed LongName() to return a fixed literal ("STEF Sorted") |
| benchmarks/benchmarks_test.go | Added chart integration calls for pdata benchmarks |
Comments suppressed due to low confidence (1)
benchmarks/encodings/stef/stef.go:93
- Changing LongName() to return the fixed literal 'STEF Sorted' (instead of e.Name()) clearly differentiates the encoding; please ensure that consumers expecting dynamic behavior are updated accordingly.
return "STEF Sorted"
| @@ -183,6 +188,9 @@ func BenchmarkSerializeFromPdata(b *testing.B) { | |||
| } | |||
|
|
|||
| func BenchmarkDeserializeToPdata(b *testing.B) { | |||
| chart.BeginChart("Deserialization To pdata Speed", b) | |||
| defer chart.EndChart("ns/point", "CPU time to serialize one data point") | |||
There was a problem hiding this comment.
In the 'BenchmarkDeserializeToPdata' function, the EndChart description still reads 'CPU time to serialize one data point'; update it to 'CPU time to deserialize one data point' to accurately reflect the benchmark's purpose.
Copilot uses AI. Check for mistakes.
Comment on lines
+12
to
+33
| <div class="item" id="mJYhGkwRaHSo" style="width:900px;height:500px;"></div> | ||
| </div> | ||
|
|
||
|
|
||
| <script type="text/javascript"> | ||
| "use strict"; | ||
| let goecharts_aLEMSbPZEhba = echarts.init(document.getElementById('aLEMSbPZEhba'), "white", { renderer: "canvas" }); | ||
| let option_aLEMSbPZEhba = {"animation":false,"color":["#92C5F9","#5470c6","#91cc75","#fac858","#ee6666","#73c0de","#3ba272","#fc8452","#9a60b4","#ea7ccc"],"legend":{},"series":[{"name":"Compressed size in bytes (zstd)","type":"bar","data":[{"value":236734,"label":{"show":true}},{"value":206885,"label":{"show":true}},{"value":549012,"label":{"show":true}},{"value":93235,"label":{"show":true}},{"value":262005,"label":{"show":true}}]}],"title":{"text":"Dataset: hipstershop-otelmetrics.zst"},"toolbox":{},"tooltip":{},"xAxis":[{"data":["Otel Arrow","Parquet","Protobuf OTLP","STEF","STEF Unsorted"]}],"yAxis":[{"name":"Bytes"}]} | ||
| let goecharts_mJYhGkwRaHSo = echarts.init(document.getElementById('mJYhGkwRaHSo'), "white", { renderer: "canvas" }); | ||
| let option_mJYhGkwRaHSo = {"animation":false,"color":["#92C5F9","#5470c6","#91cc75","#fac858","#ee6666","#73c0de","#3ba272","#fc8452","#9a60b4","#ea7ccc"],"legend":{},"series":[{"name":"Compressed size in bytes (zstd)","type":"bar","data":[{"value":236734,"label":{"show":true}},{"value":211051,"label":{"show":true}},{"value":549012,"label":{"show":true}},{"value":93235,"label":{"show":true}},{"value":262005,"label":{"show":true}}]}],"title":{"text":"Dataset: hipstershop-otelmetrics.zst"},"toolbox":{},"tooltip":{},"xAxis":[{"data":["Otel Arrow","Parquet","Protobuf OTLP","STEF Sorted","STEF Unsorted"]}],"yAxis":[{"name":"Bytes"}]} | ||
|
|
||
| goecharts_aLEMSbPZEhba.setOption(option_aLEMSbPZEhba); | ||
| goecharts_mJYhGkwRaHSo.setOption(option_mJYhGkwRaHSo); | ||
| </script> | ||
| <div class="container"> | ||
| <div class="item" id="drbjVhKNTUZd" style="width:900px;height:500px;"></div> | ||
| <div class="item" id="QXxJVEdgMuGz" style="width:900px;height:500px;"></div> | ||
| </div> | ||
|
|
||
|
|
||
| <script type="text/javascript"> | ||
| "use strict"; | ||
| let goecharts_drbjVhKNTUZd = echarts.init(document.getElementById('drbjVhKNTUZd'), "white", { renderer: "canvas" }); | ||
| let option_drbjVhKNTUZd = {"animation":false,"color":["#92C5F9","#5470c6","#91cc75","#fac858","#ee6666","#73c0de","#3ba272","#fc8452","#9a60b4","#ea7ccc"],"legend":{},"series":[{"name":"Compressed size in bytes (zstd)","type":"bar","data":[{"value":205821,"label":{"show":true}},{"value":130650,"label":{"show":true}},{"value":846035,"label":{"show":true}},{"value":83006,"label":{"show":true}},{"value":293520,"label":{"show":true}}]}],"title":{"text":"Dataset: hostandcollector-otelmetrics.zst"},"toolbox":{},"tooltip":{},"xAxis":[{"data":["Otel Arrow","Parquet","Protobuf OTLP","STEF","STEF Unsorted"]}],"yAxis":[{"name":"Bytes"}]} | ||
| let goecharts_QXxJVEdgMuGz = echarts.init(document.getElementById('QXxJVEdgMuGz'), "white", { renderer: "canvas" }); | ||
| let option_QXxJVEdgMuGz = {"animation":false,"color":["#92C5F9","#5470c6","#91cc75","#fac858","#ee6666","#73c0de","#3ba272","#fc8452","#9a60b4","#ea7ccc"],"legend":{},"series":[{"name":"Compressed size in bytes (zstd)","type":"bar","data":[{"value":205821,"label":{"show":true}},{"value":134558,"label":{"show":true}},{"value":846035,"label":{"show":true}},{"value":83006,"label":{"show":true}},{"value":293520,"label":{"show":true}}]}],"title":{"text":"Dataset: hostandcollector-otelmetrics.zst"},"toolbox":{},"tooltip":{},"xAxis":[{"data":["Otel Arrow","Parquet","Protobuf OTLP","STEF Sorted","STEF Unsorted"]}],"yAxis":[{"name":"Bytes"}]} | ||
|
|
||
| goecharts_drbjVhKNTUZd.setOption(option_drbjVhKNTUZd); | ||
| goecharts_QXxJVEdgMuGz.setOption(option_QXxJVEdgMuGz); |
There was a problem hiding this comment.
[nitpick] Consider using more descriptive container IDs instead of opaque hash-like strings to improve readability and future maintenance.
Copilot uses AI. Check for mistakes.
dmitryax
approved these changes
May 27, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These are interesting because they represent better what happens when STEF is used in Otel Collector.