Conversation
There was a problem hiding this comment.
Pull request overview
Refactors XDR array decoding to build results via array literals and push() rather than pre-sizing with new Array(length), affecting how decoded arrays are allocated/filled.
Changes:
- Update
VarArray.read()to initialize with[]and append elements withpush(). - Update fixed-length
Array.read()similarly, removing the pre-allocation comment and using[]+push().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/var-array.js |
Switch decoded VarArray result construction from pre-sized array + index assignment to [] + push(). |
src/array.js |
Switch decoded fixed-length Array result construction from new global.Array(this._length) + index assignment to [] + push(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const result = []; | ||
| // read values | ||
| for (let i = 0; i < this._length; i++) { | ||
| result[i] = this._childType.read(reader, remainingDepth - 1); | ||
| result.push(this._childType.read(reader, remainingDepth - 1)); |
There was a problem hiding this comment.
PR description claims this change improves memory performance for large arrays, but the diff switches from pre-sizing to incremental push, which changes allocation/growth behavior and may affect perf/memory depending on engine and size. Consider updating the PR description to reflect the actual motivation (e.g., avoiding holey arrays) and/or include benchmark results to support the stated memory claim.
new Array(length)to[]which results in better memory performance with large array sizes.