Replace Buffer with Uint8Array #650
Conversation
bjornstar
force-pushed
the
goodbye-nodejs-buffer
branch
from
045717d to
8395e54
Compare
|
Thank you for merging #652, I have rebased and the CI has passed 🎉 |
|
The most reliable test, if everything is backwards compatible, is to run music-metadata tests with this implementation. If those pass, I am confident we are on the right track. This diagram can be useful to understand how the dependencies are linked: https://github.com/Borewit/music-metadata?tab=readme-ov-file#dependencies I have no time to work in this today, but I can find some time this weekend. |
Oof, there's a lot of Buffer usage in there. Let me see what I can do. |
| decode(new Uint8Array(new Array(128).fill(0).map((_, index) => index))), | ||
| '\0\x01\x02\x03\x04\x05\x06\x07\b\t\n\x0B\f\r\x0E\x0F\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1A\x1B\x1C\x1D\x1E\x1F !"#$%&\'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~\x7F', | ||
| 'U+0000 to U+007F remain unchanged' | ||
| ); | ||
| assert.equal( | ||
| decode('\x80\x81\x82\x83\x84\x85\x86\x87\x88\x89\x8A\x8B\x8C\x8D\x8E\x8F\x90\x91\x92\x93\x94\x95\x96\x97\x98\x99\x9A\x9B\x9C\x9D\x9E\x9F\xA0\xA1\xA2\xA3\xA4\xA5\xA6\xA7\xA8\xA9\xAA\xAB\xAC\xAD\xAE\xAF\xB0\xB1\xB2\xB3\xB4\xB5\xB6\xB7\xB8\xB9\xBA\xBB\xBC\xBD\xBE\xBF\xC0\xC1\xC2\xC3\xC4\xC5\xC6\xC7\xC8\xC9\xCA\xCB\xCC\xCD\xCE\xCF\xD0\xD1\xD2\xD3\xD4\xD5\xD6\xD7\xD8\xD9\xDA\xDB\xDC\xDD\xDE\xDF\xE0\xE1\xE2\xE3\xE4\xE5\xE6\xE7\xE8\xE9\xEA\xEB\xEC\xED\xEE\xEF\xF0\xF1\xF2\xF3\xF4\xF5\xF6\xF7\xF8\xF9\xFA\xFB\xFC\xFD\xFE\xFF'), | ||
| decode(new Uint8Array(new Array(128).fill(0).map((_, index) => index + 128))), |
There was a problem hiding this comment.
You could use Array#from with the mapFn argument to avoid Array#fill. This is a bit more performant.
new Uint8Array(Array.from({length: 128}, (_, index) => index))There was a problem hiding this comment.
Thanks, great suggestion!
There was a problem hiding this comment.
Unfortunately, it seems to be about 10x slower:
console.time('Array.from')
for (let i = 0; i < 100000; i += 1) {
Array.from({ length: 128 }, (_, index) => index)
}
console.timeEnd('Array.from')
console.time('new Array')
for (let i = 0; i < 100000; i += 1) {
new Array(128).fill(0).map((_, index) => index)
}
console.timeEnd('new Array')Results from node:
Array.from: 371.006ms
new Array: 34.782ms
Results from chromium:
Array.from: 418.404052734375 ms
new Array: 28.22607421875 ms
There was a problem hiding this comment.
You are correct; Array.from is significantly slower on Chromium. I had only tested it on Firefox previously.
Since Array.from is only marginally faster in Firefox but considerably slower on Chromium, using new Array is the better choice overall.
|
Please rebase @bjornstar |
bjornstar
force-pushed
the
goodbye-nodejs-buffer
branch
from
d3f5914 to
4d6268c
Compare
Done! |
|
Can you please rebase one more time @bjornstar? Please ensure |
|
I'll be able to take care of it tomorrow, away from a computer at the moment. |
Co-authored-by: Zefir <will.duncan.nn+github@gmail.com>
bjornstar
force-pushed
the
goodbye-nodejs-buffer
branch
from
4d6268c to
d784434
Compare
|
@Borewit Rebased! 😃 |
|
Part of v6.0.0 |
Replaced Buffer usage with equivalent Uint8Array usage. One nice benefit was replacing the AnsiString implementation with TextDecoder 😁.
I removed the BufferType so this should be considered a breaking change.
It would be great to get a new major release so that we can incorporate these changes into sindresorhus/file-type#633