buffer: remove MAX_SAFE_INTEGER check on length#14131
Uh oh!
There was an error while loading. Please reload this page.
Conversation
MAX_SAFE_INTEGER is millions of times larger than the largest buffer allowed in Node.js. There is no need to squash the length down to MAX_SAFE_INTEGER. Removing that check results in a small but statistically significant increase for Buffer.from() operating on ArrayBuffers in some situations.
Trott commented Jul 8, 2017
improvement confidence p.value buffers/buffer-from.js n=2048 len=10 source="arraybuffer-middle" 1.54 % ** 0.001666215 buffers/buffer-from.js n=2048 len=2048 source="arraybuffer-middle" 1.76 % ** 0.003719416 |
Trott commented Jul 8, 2017
If you do somehow manage to get Before, if you manage to go here, say with This seems more correct to me. You're getting the actual length that is problematic rather than some other length that you didn't specify. |
Trott commented Jul 8, 2017
mscdex commented Jul 8, 2017
Is this technically semver-major due to the possible error change for large values? |
bnoordhuis commented Jul 8, 2017
No, IMO. |
Trott commented Jul 8, 2017
refack commented Jul 8, 2017
Depends on resolution of #13937 |
refack commented Jul 8, 2017
A ~1.5% of the time was used for that |
Trott commented Jul 11, 2017
I agree. If the wording changed, then yes. But since it's just correcting the (IMO) wrong value repeated back to the user, I would say no.
I don't think so because that's about changes after we move to the new internal error system. This one is not part of the new internal error system. |
Trott commented Jul 12, 2017
Landed in e6e6b07 |
MAX_SAFE_INTEGER is millions of times larger than the largest buffer allowed in Node.js. There is no need to squash the length down to MAX_SAFE_INTEGER. Removing that check results in a small but statistically significant increase for Buffer.from() operating on ArrayBuffers in some situations. PR-URL: #14131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
MAX_SAFE_INTEGER is millions of times larger than the largest buffer allowed in Node.js. There is no need to squash the length down to MAX_SAFE_INTEGER. Removing that check results in a small but statistically significant increase for Buffer.from() operating on ArrayBuffers in some situations. PR-URL: #14131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
MAX_SAFE_INTEGER is millions of times larger than the largest buffer allowed in Node.js. There is no need to squash the length down to MAX_SAFE_INTEGER. Removing that check results in a small but statistically significant increase for Buffer.from() operating on ArrayBuffers in some situations. PR-URL: #14131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
MAX_SAFE_INTEGER is millions of times larger than the largest buffer allowed in Node.js. There is no need to squash the length down to MAX_SAFE_INTEGER. Removing that check results in a small but statistically significant increase for Buffer.from() operating on ArrayBuffers in some situations. PR-URL: #14131 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins commented Aug 16, 2017
Should this be backported to |
Trott commented Aug 16, 2017
@MylesBorins Did you mean to label this either |
Trott commented Aug 16, 2017
(Changed to backport requested..) |
Trott commented Aug 16, 2017
@MylesBorins If #12361 lands in v6.x-staging, then this should land and will land cleanly. If #12361 does not land in v6.x-staging, then neither should this. #12361 is currently marked for |
11 participants
MAX_SAFE_INTEGER is millions of times larger than the largest buffer
allowed in Node.js. There is no need to squash the length down to
MAX_SAFE_INTEGER. Removing that check results in a small but
statistically significant increase for Buffer.from() operating on
ArrayBuffers in some situations.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
buffer