perf: improve non-SIMD with wordwise validation #123
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is awesome! Do you know how new of a compiler it requires? The MSRV CI job is grumpy 🤷 . |
Sorry for the delayed response, I've been traveling. As @paolobarbolini pointed out the new MSRV would be 1.44, though we could avoid an MSRV bump and stick to 1.36 with some extra or less-idomatic code (see latest commit) |
|
@seanmonstar It would be great to land #122 (the benchmarks & baseline) first, then I can polish this a little more before merging and releasing |
AaronO
force-pushed
the
perf/wordwise-validation
branch
from
873fa21
to
f9ccca3
Compare
|
@seanmonstar Update here: ConclusionWith the latest changes this PR nearly doubles throughput on ARM, with no real downsides. On x86_64 non-SIMD is now roughly on-par with SIMD in fact faster for smaller reqs/resps. BenchmarksM1 Airmasterperf/wordwise-validationGH Codespace 4-core SIMD(Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz) masterperf/wordwise-validationGH Codespace 4-core no-SIMDmasterperf/wordwise-validation |
|
We're eagerly waiting for this PR in Deno 🙏 |
|
Revisiting this, I took a look at the SIMD code and noticed inefficiencies on larger inputs and with a few tweaks see 2-3x improvements for header-values/URIs greater than 128b, will submit another PR. 128b+ header-values are more common in production than you would expect (and obviously we would want to scale gracefully anyway), I don't have hard data on real-world distributions but anecdotally checked my Chrome headers on Google.com, Cookie is 1024b+ and a handful are 128b+ Summary table(Disclaimer: aggregated by ChatGPT, which "computed" the ratio rows which aren't exactly correct but close enough)
Raw benches |
seanmonstar
requested changes
Apr 13, 2023
A 1st pass at improving non-SIMD perf. On aarch64/M1 we observe a ~5x asymptotic improvement in uri parsing and ~2.5x in header names & values. We also observe a -40% time reduction in the `req/req` bench. I briefly benched it on x86_64, we see a -20% time reduction in the `req/req` bench there for non-SIMD, needs more testing on x86_64 and possibly only enable these fastpaths if SIMD is off
AaronO
force-pushed
the
perf/wordwise-validation
branch
from
cf7933e
to
2fe5407
Compare
|
@seanmonstar I would consider this PR good to land now, I'll start another PR with the SIMD improvements. I have identified 2-3 other improvements that will help with "latency" and a cleanup, will try to get to those time permitting. |
|
@seanmonstar The SIMD-related PR is up here: #131 |
|
Small update, I've got a first-pass implementation of NEON support (based off this #123 and #132), the bench results are promising (M1 Air): I implemented a vectorized header-name validator but without other changes it's a net-negative since in practice header-names are often less than 16 bytes. Benches with vectorized header-name: I do think there's room for improvement, specifically the handoff between SIMD and non-SIMD is not ideal right now. |
Merged
|
@seanmonstar NEON PR is up: #133 The PRs mostly build off each other and can be landed/reviewed in series. |
seanmonstar
approved these changes
Apr 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A 1st pass at improving non-SIMD perf, to land after #122
On aarch64/M1 we observe a ~5x asymptotic improvement in uri parsing and ~2.5x in header names & values.
We also observe a -40% time reduction in the
req/reqbench.I briefly benched it on x86_64, we see a -20% time reduction in the
req/reqbench there for non-SIMD, needs more testing/tweaks on x86_64 and possibly only enabling these fastpaths if SIMD is offBench results
Prev baseline (from #122)