src: make base64 decoding 10-15% faster#2193
Uh oh!
There was an error while loading. Please reload this page.
Conversation
mscdex added the c++ 
src/string_bytes.cc Outdated
There was a problem hiding this comment.
very minor nit. Is four spaces here, okay?
There was a problem hiding this comment.
Should we really cast the result to uint8_t when the actual data is int8_t?
There was a problem hiding this comment.
Yes. The alternative was to change the type of the list elements to uint8_t but then I'd also have to change all the -1 to 255 to squelch compiler warnings. It would make the diff a lot noisier for no reason; conversion from signed to unsigned is well-defined.
There was a problem hiding this comment.
Is four spaces here, okay?
I don't think we really have a convention for that but I'll change it to two spaces before landing.
bnoordhuisforce-pushed the optimize-base64-decode branch from 7f8acb6 to a5df468Comparebnoordhuis commented Jul 17, 2015
Incorporated feedback, PTAL. @trevnorris Good suggestion about force-flattening the string first. I'm confident saying now that it's actually 50% faster. :-) |
trevnorris commented Jul 17, 2015
LGTM |
ronkorving commented Jul 21, 2015
@bnoordhuis then I guess you can rename this PR :) great job! |
Make the inner loop execute fewer compare-and-branch executions per processed byte, resulting in a 50% or more speedup. This coincidentally fixes an out-of-bounds read: while (unbase64(*src) < 0 && src < srcEnd) Should have read: while (src < srcEnd && unbase64(*src) < 0) But this commit removes the offending code altogether. Fixes: nodejs#2166 PR-URL: nodejs#2193 Reviewed-By: Trevor Norris <[email protected]>
parallel/test-buffer called `Buffer.prototype.toString()` on a buffer with uninitialized memory. Call `Buffer.prototype.fill()` on it first. PR-URL: nodejs#2193 Reviewed-By: Trevor Norris <[email protected]>
bnoordhuisforce-pushed the optimize-base64-decode branch from a5df468 to ac70bc8Comparebnoordhuis commented Jul 25, 2015
Landed in 8fd3ce1 and ac70bc8 with hex values, thanks everyone. I only added @trevnorris in the Reviewed-By because he was the only one to formally LGTM it. |
YurySolovyov commented Jul 25, 2015
Is it going to be in 2.5.0, 3.0 or both? |
Fishrock123 commented Jul 26, 2015
@YuriSolovyov 2.5.0+ (both) :) |
10 participants
Make the inner loop execute fewer compare-and-branch executions per
processed byte, resulting in a 10-15% speedup.
This coincidentally fixes an out-of-bounds read:
Should have read:
But this commit removes the offending code altogether.
Fixes: #2166
R=@trevnorris?
CI: https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/155/