perf(util-endpoints): reduce temporary object allocations#1957
perf(util-endpoints): reduce temporary object allocations#1957
Conversation
trivikr
changed the title
trivikr
changed the title
…ent in evaluateEndpointRule
trivikr
force-pushed
the
remove-object-allocation-evaluate-condition
branch
from
b96ce29 to
6a00f07
Compare
trivikr
force-pushed
the
remove-object-allocation-evaluate-condition
branch
from
ccc8015 to
78eb850
Compare
| const conditionOptions: EvaluateOptions = { | ||
| ...options, | ||
| referenceRecord: { ...options.referenceRecord }, | ||
| }; |
There was a problem hiding this comment.
is it safe to use options all the way through with no copying?
Why I think it might be safe: options object lifecycle is one endpoint resolution, and this is all synchronous code...
There was a problem hiding this comment.
I'll avoid modifying options, as it might create issues in future. It can be tested in a separate PR.
| endpointToReturn.properties = getEndpointProperties(properties, endpointRuleOptions); | ||
| } | ||
|
|
||
| return endpointToReturn; |
There was a problem hiding this comment.
unlikely, but does the call order matter for exception handling?
the old code called getEndpointUrl last.
There was a problem hiding this comment.
Actually, the previous code would have thrown exceptions from getEndpointHeaders and getEndpointProperties before those from getEndpointUrl.
It should be okay though.
| ...options, | ||
| referenceRecord: { ...options.referenceRecord, ...referenceRecord }, | ||
| }); | ||
| const treeRuleOptions = referenceRecord |
There was a problem hiding this comment.
the more code I (re-)read, the more I think options is safe to mutate
2 participants
Issue
Internal JS-6645
Description
Remove rest spread destructuring that created a new object on every call, and replace conditional spread in the return with an early return branch to avoid temporary object allocation.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.