perf: SIMD percent_encode against the character-set bitmap - #1230
Merged
Conversation
Scan and encode 16-byte runs with SSSE3 pshufb, NEON tbl, or RVV indexed loads. Tables come from the existing 32-byte character_set bitmap, so short strings keep the inline 8-byte scalar path and avoid the per-call LUT build that regressed #1124. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
Merging this PR will not alter performance
Performance Changes
Comparing Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1230 +/- ##
==========================================
+ Coverage 63.00% 63.25% +0.25%
==========================================
Files 38 39 +1
Lines 7628 7699 +71
Branches 3496 3514 +18
==========================================
+ Hits 4806 4870 +64
Misses 749 749
- Partials 2073 2080 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Use the equivalent _mm_cmpgt_epi8 form instead of the deprecated _mm_cmplt_epi8 compare when selecting the high half of the bitmap. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
anonrig
marked this pull request as ready for review
August 21, 2026 17:12
lemire
approved these changes
Aug 21, 2026
Restore the original inline 8-byte percent_encode_index scan so username/hash setters do not pay an extra branch or out-of-line call. Use SIMD encode only when the remaining suffix is at least 48 bytes, which is past the CodSpeed setter and UserInfo inputs that regressed from table setup and dense mask walking. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
The just_ascii test rejects non-ASCII source; replace the en-dash in the SIMD threshold comment. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
Keep the original std::ranges::find_if prefix check so SetHash and other no-encode setter paths match main on instruction count. SIMD still runs only for remaining suffixes of 48 bytes or more. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
cursor Bot
pushed a commit
that referenced
this pull request
Aug 21, 2026
CodSpeed on #1218/#1230 showed 16-byte encode classify regresses SetHash and the official percent_encode examples. Gate the nibble-table walk on a 48-byte remainder and keep it noinline so setter-sized percent_encode stays a tight scalar tail. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
SetHash still regressed on CodSpeed after the 48-byte SIMD gate because the template inlined the SIMD dispatch. Restore the original find_if plus byte loop in the append/replace template so hash/search setters match main. SIMD stays on the allocating overloads used by long query/fragment strings. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
SetHash and SetPort still moved on CodSpeed after the scalar setter template was restored, because the SSSE3 kernel lived in the ada.cpp unity TU and changed inlining. Move the kernel to unicode_percent_encode.cpp and keep short encode loops in unicode.cpp. Single-header builds still amalgamate the kernel through ada.cpp. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
Classify 32 bytes per iteration with vpshufb when AVX2 is available (runtime dispatch from an SSE2 TU). SSSE3 and NEON now append a fully clean 32-byte pair in one go. Reserve 3x the remaining suffix so long dirty strings do not realloc while walking windows. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
Apple Clang Release inlines the setter template and omits its symbol, so macOS static-library tests fail to link. Explicitly instantiate both append modes. clang-cl treats unused SIMD helpers as errors when SSSE3 is disabled; compile those helpers only for SSSE3 and NEON. Drop AVX2 runtime dispatch. CodSpeed reported setter regressions on that revision; the previous separate-TU commit without AVX2 did not. Co-authored-by: Yagiz Nizipli <anonrig@users.noreply.github.com>
FranciscoThiesen
added a commit
to FranciscoThiesen/ada
that referenced
this pull request
Aug 26, 2026
…identical to main) Including the ~1800-line route-set compiler in the unity ada.cpp reshuffles GCC's unit-wide inlining budget: url_aggregator setter hot paths lose inlined std::string growth (e.g. append_base_pathname 200 -> 106 instructions), which CodSpeed reported as SetHash -12%, SetProtocol -6%, SetPort -5%, SetHostname -3%. Clang is unaffected. Mirror the ADA_PERCENT_ENCODE_SIMD_SEPARATE_TU pattern from ada-url#1230: build url_pattern_list.cpp as a separate TU under CMake (ADA_URL_PATTERN_LIST_SEPARATE_TU) while the amalgamated single-file build keeps including it inline. With the guard active the unity TU's pre-existing functions are opcode-identical to main (0 of 793 changed, GCC 14 -O3). Also drops an unused constant that the standalone TU surfaced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or 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
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.
Summary
percent_encodewas still a scalarfind_ifplus byte loop on the parse / setter /url_search_paramshot paths.This adds a 16-byte encode kernel, while keeping setter codegen on the original scalar code:
pshufblooks up the existing 32-bytecharacter_setbitmap (cs[b >> 3] & (1 << (b & 7))). gcc/clang x86-64 usestarget("ssse3")so a baseline SSE2 build still gets the kernel, matchingparser.cpp. Clean 32-byte pairs are appended in one go.vqtbl2q_u8over the same bitmap, also pairing clean 32-byte runs.vluxei8).%XXonly for set bits. The suffix output is reserved at 3x remaining length to avoid realloc.Short / setter paths stay on the original code in the
ada.cppunity TU:percent_encode_indexis the original inline 8-byte unroll.percent_encodetemplate (used byset_hash/set_search) is the originalfind_ifplus byte loop.percent_encodeoverloads use that same scalar loop when the remaining suffix is under 48 bytes.unicode_percent_encode.cpp(its own TU) so it does not change setter inlining. Single-header builds still amalgamate it throughada.cpp.No per-call nibble LUT (the #1124 regression).
Why this path
Issue #1120 identified percent-encode as the remaining scalar bottleneck. Host/path/query/hash scans are already SIMD; encode was still byte-at-a-time on long query/fragment/form strings.
Performance
Release, g++ 13.3, Xeon. Long query/fragment (~200 B) uses the SIMD suffix. Setter microbenchmarks match
main.CodSpeed
Earlier revisions inlined SIMD into the unity TU and regressed SetHash. Setter templates stay on the scalar loop; the kernel is a separate TU. AVX2 runtime dispatch was dropped after it moved unrelated setter benches.
Correctness
Oracle tests for every official character set, lengths 0–80 plus 96/128/256-byte windows, encode bytes at every offset, non-ASCII, dense punctuation, and append/replace templates.
The
percent_encode<append>template is explicitly instantiated so Apple Clang Release still exports the symbol for tests.No public signature or object-layout change.
Closes #1120.