perf(otlp-transformer): optimize toAnyValue with pre-allocated arrays#6287
Conversation
Replace .map() with pre-allocated arrays and for loops for better performance when transforming arrays and objects. Benchmark results show significant improvements: - Array of objects: 3.5M -> 9.2M ops/sec (+160%) - Nested objects: 4.1M -> 12.2M ops/sec (+196%) - Complex nested: 1.1M -> 2.2M ops/sec (+102%)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6287 +/- ##
=======================================
Coverage 95.58% 95.59%
=======================================
Files 314 314
Lines 9590 9596 +6
Branches 2221 2221
=======================================
+ Hits 9167 9173 +6
Misses 423 423
🚀 New features to boost your workflow:
|
| if (t === 'boolean') return { boolValue: value as boolean }; | ||
| if (value instanceof Uint8Array) return { bytesValue: value }; | ||
| if (Array.isArray(value)) | ||
| return { arrayValue: { values: value.map(toAnyValue) } }; |
There was a problem hiding this comment.
Although v8 is normally great at handling Array.map, I'm guessing the recursion messes up how v8 can optimize the array allocation. The recursion in combination with the new closure also might cause issues. Still kind of surprised at this change though, was expecting to only have to adjust the object conditional.
I'm pretty sure the performance benefit from the object if case is just removing the usage of Object.entries (and the extra [k, v] destructure in .map().
pichlermarc
left a comment
There was a problem hiding this comment.
thanks! this looks great - the benchmarks are helpful for review :)
Results on my machine:
Benchmark results (Apple M3 Pro, Node.js 20.18)
**old:**> @opentelemetry/otlp-transformer@0.210.0 test:bench
> node test/performance/benchmark/micro-benchmarks.js && node test/performance/benchmark/index.js | tee .benchmark-results.txt
toAnyValue string x 1,556,027,000 ops/sec ±0.37% (94 runs sampled)
toAnyValue integer x 164,810,203 ops/sec ±0.47% (101 runs sampled)
toAnyValue array of strings x 33,552,094 ops/sec ±0.16% (99 runs sampled)
toAnyValue array of objects x 4,208,818 ops/sec ±0.20% (99 runs sampled)
toAnyValue nested object x 4,840,973 ops/sec ±0.15% (98 runs sampled)
toAnyValue complex nested x 1,274,070 ops/sec ±0.22% (101 runs sampled)
transform 1 span x 1,794,993 ops/sec ±0.24% (95 runs sampled)
transform 100 spans x 23,486 ops/sec ±0.16% (99 runs sampled)
transform 100 spans to protobuf x 3,364 ops/sec ±0.22% (100 runs sampled)
new:
> @opentelemetry/otlp-transformer@0.209.0 test:bench
> node test/performance/benchmark/micro-benchmarks.js && node test/performance/benchmark/index.js | tee .benchmark-results.txt
toAnyValue string x 1,633,555,952 ops/sec ±0.67% (94 runs sampled)
toAnyValue integer x 170,046,965 ops/sec ±0.19% (95 runs sampled)
toAnyValue array of strings x 33,488,015 ops/sec ±0.28% (96 runs sampled)
toAnyValue array of objects x 13,248,048 ops/sec ±0.31% (99 runs sampled)
toAnyValue nested object x 18,235,656 ops/sec ±0.85% (99 runs sampled)
toAnyValue complex nested x 2,729,235 ops/sec ±0.82% (98 runs sampled)
transform 1 span x 1,924,045 ops/sec ±0.25% (97 runs sampled)
transform 100 spans x 25,347 ops/sec ±0.13% (100 runs sampled)
transform 100 spans to protobuf x 3,555 ops/sec ±0.20% (100 runs sampled)
One note though about them being included in the public docs, see comment below. Other than that this PR is good to go.
| ### :house: Internal | ||
|
|
||
| * chore(browser): fix CODEOWNERS paths for browser-related packages | ||
| * perf(otlp-transformer): optimize toAnyValue performance [#6287](https://github.com/open-telemetry/opentelemetry-js/pull/6287) @AbhiPrasad |
There was a problem hiding this comment.
please move this to experimental/CHANGELOG.md
|
@pichlermarc thanks for the review! I've adjusted the benchmarks and fixed the changelog accordingly. |
Which problem is this PR solving?
Update the
toAnyValuein theotlp-transformerpackage to replace .map() with pre-allocated arrays and for loops for better performance when transforming arrays and objects.Type of change
Performance Improvement
How Has This Been Tested?
Add a new benchmark for this (AI generated). Tested on a 2021 Apple M1 Pro 32 GB - MacOS 15.5 (24F74)
Benchmark results show significant improvements:
Checklist: