Buffer speedups #1048
Buffer speedups #1048
Conversation
|
https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/239/nodes=iojs-armv7-ubuntu1401/ https://jenkins-iojs.nodesource.com/view/iojs/job/iojs+any-pr+multi/234/nodes=iojs-armv7-ubuntu1401/ takes longer on there, 12 mins vs 11 mins - note this machine is using ccache so builds are super fast. |
| 'size: 0x' + kMaxLength.toString(16) + ' bytes'); | ||
| // Slightly less common case. | ||
| if (typeof(arg) === 'string') { | ||
| fromString(this, arg, arguments.length > 1 ? arguments[1] : 'utf8'); |
There was a problem hiding this comment.
why do a length check here just to check if encoding is a string in fromString()?
There was a problem hiding this comment.
Two reasons, one aesthetic, one technical. Aesthetic: it matches the check at the top of the function. Technical: an unguarded arguments[1] is megamorphic whereas the length check keeps the property/index lookups monomorphic.
I didn't benchmark it to the death but I did look at the generated code and came to the conclusion that monomorphic is best, even with the additional property lookup. I don't know if you have looked at the machine code for KeyedLoadIC_Megamorphic but it's pretty complex.
|
My perf results on x86_64 Linux: Minor perf improvement and code is more readable. Just have a couple questions, but LGTM. |
bnoordhuis
force-pushed
the
buffer-speedups
branch
from
237c67f
to
79facc3
Compare
|
@trevnorris Updated, PTAL. @rvagg I'm not sure if I would trust the CI for anything related to benchmarking. |
|
@bnoordhuis "reliable" CI benchmarking is planned down the road |
|
too bad about needing to use |
The Buffer constructor is used pervasively throughout io.js, yet it was
one of the most unwieldy functions in core. This commit breaks up the
constructor into several small functions in a way that makes V8 happy.
About 8-10% CPU time was attributed to the constructor function before
in buffer-heavy benchmarks. That pretty much drops to zero now because
V8 can now easily inline it at the call site. It shortens the running
time of the following simple benchmark by about 15%:
for (var i = 0; i < 25e6; ++i) new Buffer(1);
And about 8% from this benchmark:
for (var i = 0; i < 1e7; ++i) new Buffer('x', 'ucs2');
PR-URL: nodejs#1048
Reviewed-By: Trevor Norris <[email protected]>
Avoid a costly String#toLowerCase() call in Buffer#write() in the
common case, i.e., that the string is already lowercase. Reduces
the running time of the following benchmark by about 40%:
for (var b = Buffer(1), i = 0; i < 25e6; ++i) b.write('x', 'ucs2');
PR-URL: nodejs#1048
Reviewed-By: Trevor Norris <[email protected]>
bnoordhuis
force-pushed
the
buffer-speedups
branch
from
79facc3
to
4ddd640
Compare
R=@trevnorris?
With these patches,
make testfinishes (for the first time ever!) in under two minutes on my MBA. It shaves off about 5 seconds from the total running time. :-)