perf(serialization): bench the actual path used by the trace exporter - #2512
perf(serialization): bench the actual path used by the trace exporter#2512paullegranddc wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
📚 Documentation Check Results📦
|
🔒 Cargo Deny Results✅ No issues found! 📦
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa652005bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| || vec![0u8; 12_000_000], | ||
| || {}, | ||
| |()| msgpack_encoder::v04::to_vec_with_capacity_from_v04(&data, 1_000_000), | ||
| criterion::BatchSize::SmallInput, |
There was a problem hiding this comment.
When Criterion performs warm-up or samples with many iterations, SmallInput retains the outputs from roughly one-tenth of those iterations until the batch completes. Every output here has at least 1,000,000 bytes of capacity, so a single batch can retain hundreds of megabytes or more, potentially causing memory pressure or OOMs that invalidate the benchmark. Use LargeInput or PerIteration for this large returned value.
Useful? React with 👍 / 👎.
| // Generate roughly 10mb of data. This is the upper bound of payload size before a tracer | ||
| // flushes | ||
| let data = rmp_serde::to_vec(&generate_trace_chunks(20, 2_075)) | ||
| let data = rmp_serde::to_vec(&generate_trace_chunks(100, 20)) |
There was a problem hiding this comment.
The benchmark still says it generates roughly 10 MB, but this change reduces the payload from 41,500 spans (20 * 2,075) to only 2,000 (100 * 20). Because each span has the same fixed fields, the resulting payload is approximately one-twentieth of the previous size, so both benchmarks stop exercising the documented upper-bound flush workload and their results are no longer comparable to the prior benchmark. Retain enough spans to approach 10 MB, or explicitly redefine the benchmark and its description if the smaller workload is intentional.
Useful? React with 👍 / 👎.
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
BenchmarksComparisonBenchmark execution time: 2026-09-11 13:14:49 Comparing candidate commit aa65200 in PR branch Found 10 performance improvements and 10 performance regressions! Performance is the same for 107 metrics, 0 unstable metrics.
|
1 participant
What does this PR do?
The existing serialization benchmarks use
write_to_slice_from_v04which goes through the blanket impl of RmpWrite for io::Write.The trace exporter uses
to_vec_with_capacity_from_v04which through a more specialized dispatch, which is more efficient, and gives different perf characteristics when trying to optimize thermpcrate.This PR adds a benchmarks exercising the function used in the trace exporter for more realistic numbers
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
How to test the change?
Describe here in detail how the change can be validated.