node-semver
node-semver copied to clipboard
[BUG] SemVer.compare causes spurious allocations of SemVer objects
Is there an existing issue for this?
- [X] I have searched the existing issues
Current Behavior
https://github.com/npm/node-semver/blob/main/functions/compare.js#L3 always creates a new SemVer objects even if you're careful about making sure you're already using SemVer objects.
It relies on https://github.com/npm/node-semver/blob/main/classes/semver.js#L14 to return the original semver from the constructor (TIL Javascript constructors can have return statements). This may work but is surprising and seems to cause a spurious allocation.
Expected Behavior
https://github.com/npm/node-semver/blob/main/functions/compare.js#L3 conditionally converts it's operands to SemVer objects and doesn't create spurious allocations.
Steps To Reproduce
const a = new SemVer("1.0");
const b = new SemVer("2.0");
// Chrome's Memory profiler will tell you that new SemVer objects we're created in this call.
a.compare(b);
Environment
- npm:
- Node: v16
- OS: Mac Os 12.3.1
- platform: Macbook Pro
Why is this a problem? "allocations" aren't observable to JavaScript; are quite unique to individual implementations, and in this case can likely be trivially JITted away.
I’m calling compare inside of what’s essentially a tight loop. The allocations are t being JITed away and garbage collection is showing up on my profiling so I’m trying to reduce allocations.
On Tue, May 10, 2022 at 1:30 AM Jordan Harband @.***> wrote:
Why is this a problem? "allocations" aren't observable to JavaScript; are quite unique to individual implementations, and in this case can likely be trivially JITted away.
— Reply to this email directly, view it on GitHub https://github.com/npm/node-semver/issues/458#issuecomment-1121948243, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQGASC3DHRCWEN4HSCTDV3VJHX5TANCNFSM5VQJLF2A . You are receiving this because you authored the thread.Message ID: @.***>
using this issue as a reference for auditing the creation of all new Semver/Range
classes when called from methods. semver
tries to reuse instances in some cases but not others. I think the ideal solution would be to have a standard way of determining options and always reusing the instance, plus the addition of a clone method (#378).