[WTF] URLParser: table-lookup SIMD classification; keep host:port and IPv4 hosts on the fast path - #454
Conversation
… dotted-quad hosts on the fast path - Classify 16 bytes with two pshufb/tbl nibble-table lookups instead of a chain of compares. The tables are computed at compile time from scanClassTable (consteval, static_assert'ed to fit), so vector and scalar classification cannot disagree. Used when SSSE3 or NEON is available; the compare-based classifiers remain as the fallback. top100: 680 -> 553 instructions/URL, 3.9 KB URLs: 7830 -> 5720. - The prefix fast path handed off to the state machine at ':' after a host and for any digit-led last label. A decimal, non-default port without a leading zero, and a canonical dotted-quad IPv4 host, produce output identical to the input, so copy them. http://localhost:3000/... style URLs: 1217 -> 658 instructions/URL; IPv4 hosts: 1082 -> 670.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 59 minutes Limit details: You’ve used all 1 included review currently available under your plan. You completed 75 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
Preview Builds
|
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it extends the URL parser's fast path for host:port and IPv4 hosts — where a divergence from the state-machine path would be a parser differential with origin/SSRF implications — a human look is still worthwhile.
What was reviewed:
parseCanonicalIPv4Addressedge cases (leading zeros, >255 octets, short/long forms, hex/octal, trailing dot) — all correctly return nullopt and fall to the general path.- Port fast-path guards:
@after digits,\\after digits, default ports, leading-zero ports, >65535, port 0, tab/newline in port — all rejected or handed to the state machine;m_portLengthmax is 6, fits the 3-bit field. - Nibble-table classifier: tables are
consteval-derived fromscanClassTablewithstatic_assert(tables.fits); pshufb/tbl indices are masked to 0–15 so the high-bit-zeroes-output quirk of_mm_shuffle_epi8doesn't apply; the narrowed sentinel bytes 0x00/0xFF are stops in every class used.
Extended reasoning...
Overview
This PR makes two independent performance changes to Source/WTF/wtf/URLParser.cpp:
- Replaces the per-block chain of SIMD compares in the run scanners with a two-table nibble lookup (
pshufb/tbl), gated on SSSE3 or NEON. The tables are built at compile time bymakeNibbleTablesdirectly fromscanClassTable, withstatic_assert(tables.fits)enforcing that ≤8 distinct high-nibble patterns exist. The compare-chain lambdas remain as the fallback for pre-SSSE3 x86. - Extends the straight-line prefix fast path so that
scheme://host:port(decimal, non-default, no leading zero) and canonical dotted-quad IPv4 hosts are copied verbatim rather than handed to the state machine.parseCanonicalIPv4Addressis factored out ofparseIPv4Hostand reused as the gate.
Security risks
URL parsing is security-critical: any input for which the fast path produces a different serialization or component boundary than the state machine is a parser differential that can lead to origin confusion or SSRF. The PR is explicitly designed so the fast path only handles inputs that serialize to themselves, and everything else falls through unchanged. I traced the adversarial cases I could think of — host:3000@evil, host:3000\\evil, default ports, :0080, :65536, hex/octal/short IPv4, trailing-dot IPv4, tab/newline inside port, foo.123 (ends-in-a-number) — and each is correctly rejected by the hasPort conjunction or the parseCanonicalIPv4Address gate and takes the pre-existing AuthorityOrHost path. The !hasPort branch's new IPv4 check at line 1718 is only reached when classes & HostNotPlain is clear, so [hostStart, p) contains only plain host characters when parseCanonicalIPv4Address is called there.
For the SIMD classifier, the risk would be a byte that classifies differently under the nibble-table path than under the scalar scanClassTable lookup used in the tail. Since the tables are derived from scanClassTable itself via consteval, they encode the identical predicate; the pshufb high-bit behaviour is neutralised by masking indices to 4 bits.
Level of scrutiny
High. This is ~200 net lines of intricate control flow and platform-specific intrinsics in a WHATWG-URL-spec-conformance path that determines origins. The author's verification (differential over ~1.4M inputs including WPT and fuzz corpora, both string widths, SSE2 fallback build, ASan libFuzzer) is strong, but the edge-case density here is exactly where a second pair of eyes on the hasPort conjunction and the findPathRunEnd rewrite is valuable.
Other factors
m_portLengthis a 3-bit bitfield (max 7); the fast path caps digits at 5 sop - hostEnd≤ 6, which fits.defaultPortreturnsUINT_MAXfor File/NonSpecial, but this fast path is only entered for special non-File schemes (line 1670 bails on those), so that sentinel is never compared against a real port here.- The
additionalStopCharactertemplate default of0folds NUL into the stop set, which is already a stop for every class used, so it's a no-op. - No prior human or bot reviews on this PR (CodeRabbit was rate-limited).
…st path Port the remaining oven-sh/WebKit#454 ideas onto current main (#1213 already keeps host:port on the prefix fast path): - Classify 16-byte runs with two pshufb/tbl nibble tables when SSSE3 or NEON is available (SSE2 compare chains remain the fallback). - Accept a canonical dotted-quad IPv4 host that serializes to exactly those characters (no trailing dot, no hex/octal/short form). - Cheap-skip other digit-led hosts so non-decimal IPv4 does not enter the SIMD scanner only to fall through. Output is intended to be byte-identical to the previous parser. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
…st path Port the remaining oven-sh/WebKit#454 ideas onto current main (#1213 already keeps host:port on the prefix fast path): - Classify 16-byte runs with two pshufb/tbl nibble tables when SSSE3 or NEON is available (SSE2 compare chains remain the fallback). - Accept a canonical dotted-quad IPv4 host that serializes to exactly those characters (no trailing dot, no hex/octal/short form). Output is intended to be byte-identical to the previous parser. Signed-off-by: Yagiz Nizipli <yagiz@nizipli.com>
Follow-up to #452. Two independent changes, output byte-identical to before:
Table-lookup SIMD classification. The run scanners classified 16 bytes with a chain of vector compares (up to ~12 per block for the host scanner). This replaces that with the two-
pshufb/tblnibble-table technique:low[b & 0xF] & high[b >> 4] != 0. High nibbles with the same 16-byte stop pattern share a bit, so any of our stop sets fits in 8 bits. The tables are built at compile time fromscanClassTableitself (consteval,static_assert(tables.fits)), so the vector and scalar classifiers can't drift apart. Enabled when SSSE3 (our x64 baseline is nehalem) or NEON is available; the compare-based lambdas remain as the fallback and were re-verified with-march=x86-64.host:port and dotted-quad hosts stay on the prefix fast path. The fast path handed off to the state machine at
:after a host, and for any host whose last label could be a number — i.e. everyhttp://localhost:3000/…andhttp://127.0.0.1:8080/…. A decimal, non-default port with no leading zero, and a canonical dotted-quad IPv4 address, serialize to exactly their input, so they can be copied. Everything else (:,:0080, default ports,:65536,host:80@evil, hex/octal/short IPv4, trailing dot, …) still takes the general path.Cycles per URL (this box,
-march=nehalem, min of 8; Ada 4.0.0 and Ada with ada-url/ada#1214 which ports #452's ideas, for reference):localhost:3000,10.0.0.5:8080, …)Instructions/URL: top100 680 → 553, long 7830 → 5720, host:port 1217 → 658, ipv4 1082 → 670.
Verification: differential old-parser-vs-new over the full corpus (~1.4M inputs incl. WPT-with-bases, Ada-derived corpora, Windows/UNC, surrogate and structured fuzz sets) identical in both string widths, also with the SSE2-only fallback build; TestWTF
*URL*(Debug, parser self-check on); libFuzzer old-vs-new differential target (ASan + asserts) clean.