-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
deps,src: use SIMD for normal base64 encoding #39775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Generally looks good but can I ask you to include a README.md or guide that describes where the dep comes from, how to update it, the latest commit sha, etc? Either that, or a makefile in |
|
@jasnell I've added a README.md. Let me know if it looks good enough. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
eed8c55 to
3d89745
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Alright, I tried with CI, but it seems to be failing on unrelated flaky tests and unreliable Jenkins connections (at least more consistently on armv7l). |
PR-URL: nodejs#39775 Reviewed-By: Luigi Pinca <[email protected]>
This is an alternative to #39701 that adds a base64 SIMD library. The gains with this solution are significantly greater than those in #39701.
To answer a question/concern from the other PR: I did compare with a build with
-march=nativeand it performed identical to the generic binary when it came to the base64 encoding benchmark, so at least gcc doesn't seem to automagically optimize like we can when explicitly using SIMD (when available).The way this base64 library works is it uses CPU feature detection at runtime on x86 and for ARM it's only enabled when
--dest-cpuis eitherarm64(where NEON is always available) orarmwith--with-arm-fpuset toneon. In all other cases a generic implementation similar to (but probably better than) the one in #39701 gets used as a fallback.For now, I'm only enabling it for normal base64 encoding. The end goal is to use the library for base64url encoding too and hopefully for base64 decoding as well. Supporting those other functions will take some work because base64url is not currently supported by the library and the library's decoding does not natively support V8's 16-bit strings and IIRC the decoder's behavior does not match that of node's decoder (e.g. ignoring whitespace or other characters).
Anyhow, here is the benchmark result on a Core i7-3770K:
and unlike in #39701, this time I had the chance to run the benchmark on an armv8 board (Odroid C2):