createNodeArray mutates its input if not already a NodeArray#20471
createNodeArray mutates its input if not already a NodeArray#20471ghost wants to merge 1 commit into
Conversation
20f1b2c to
d9d9c7f
Compare
d9d9c7f to
dff9578
Compare
| } | ||
|
|
||
| const array = <NodeArray<T>>elements; | ||
| const array = elements as MutableNodeArray<T>; |
There was a problem hiding this comment.
Perhaps, if its not already a NodeArray, we should instead clone it via .slice() before modifying it? Then we wouldn't need the Nodes<T> type, though at first glance I'm not certain what the memory/performance impact would be.
There was a problem hiding this comment.
I tested out adding slice inside of createNodeArray (at the bottom of the if (elements) block), and actually saw a slight decrease in emit time for some reason -- might have to do with the fact that createNodeArray would otherwise have to change the shape of an array object acquired from elsewhere, but if we create a fresh slice it may be allocated with the correct shape already.
The most efficient option may be to avoid ever calling createNodeArray on an existing array, and instead always start by calling createNodeArray to get an empty mutable array, then pushing to it.
|
Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it. |
Fixes #20429 (comment)
The input to
createNodeArrayshould not be aReadonlyArraysince it's about to be cast toNodeArrayand then mutated. Combined withemptyArraythis was mutating global state.