perf(otlp-transformer): reduce allocations #4578
Conversation
| @@ -17,7 +17,11 @@ import type { IAnyValue, IKeyValue } from './types'; | |||
| import { Attributes } from '@opentelemetry/api'; | |||
|
|
|||
| export function toAttributes(attributes: Attributes): IKeyValue[] { | |||
| return Object.keys(attributes).map(key => toKeyValue(key, attributes[key])); | |||
| const result = []; | |||
| for (const key in attributes) { | |||
There was a problem hiding this comment.
using x in y can have issues with some runtimes, it might still be better to use Object.keys and just avoid the map call.
There was a problem hiding this comment.
In what runtimes? I though this is such a fundamental feature, tbh. The downside is that Object.keys will allocate array
There was a problem hiding this comment.
I don't remember the runtime, as it was around 2 years ago and it was caused (for Application Insights) by the runtime "adding" additional properties to the prototype of Object and Array which when accessed caused and exception. Let me see if I can find the PR and if I added comments for the details.
There was a problem hiding this comment.
- AI - #1420 Fix issues with for..in usage with prototype extension libraries like ember.js and prototype.js
There was a problem hiding this comment.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties#querying_object_properties
Here what I found in docs, so I used the strictest version propertyIsEnumerable()
CHANGELOG.md
Outdated
| @@ -11,6 +11,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/ | |||
|
|
|||
| ### :rocket: (Enhancement) | |||
|
|
|||
| * perf(otlp-transfromer): reduce allocations | |||
There was a problem hiding this comment.
this should go in experimental/CHANGELOG.md
Which problem is this PR solving?
The penguins are losing homes
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Just usual low hanging fruits - do not iterate map via entries(), use regular loops, less array copying via map
Fixes # (issue)
Short description of the changes
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: