Add URLPatternList - #166
lucacasonato wants to merge 3 commits into
Conversation
This commit adds support for a URLPatternList class.
wanderview
left a comment
There was a problem hiding this comment.
Thanks for looking at this!
| <xmp class="idl"> | ||
| [Exposed=(Window,Worker)] | ||
| interface URLPatternList { | ||
| constructor(sequence<URLPatternListEntry> entries); |
There was a problem hiding this comment.
I think some of the feedback in #30 suggested it would be nice to be able to dynamically mutate the list. So add new entries to the list and possibly delete entries from the list.
There was a problem hiding this comment.
Makes sense. I didn't want to add this right away until we figure out the internal sort order a little better.
|
Also:
I doubt the API shape here will really dictate whether a radix tree implementation is possible or not. Instead I expect the path-to-regexp style pattern matching will impose some constraints which make a pure radix solution difficult. That does not mean optimizations will not be possible, though. I think there are clear optimizations a list object like this can enable; e.g. log(n) searching of a fixed string prefix in the patterns. I have also theorized it might be possible to construct an optimization where the patterns are deconstructed into automata similar to regex compilation. Then something akin to a radix tree could be built from the automata. We could then evaluate the automata against the input in O(n) time where n is the input length. |
|
Yeah, that makes total sense. My first pass was going to just deal with speeding up fixed text prefix matches (actually also for regular
I was also thinking about that today! :) I was actually planning to do exactly this for a v2 of the What do you think about including this intermediate format in the spec? Doing that would allow us to make sure future spec additions / changes don't break non-regexp matcher algorithms. The spec would still only specify the naive regexp based matcher, but it'd be much easier for implementations to implement alternative faster matchers based on this intermediate format. |
I don't think we should put any intermediate representation in the spec as it then makes future optimizations more difficult in the implementation. I think we want the flexibility to change the intermediate representation in the future. Also, none of this stuff should be observable to sites. |
|
Also, when this is further along it would be nice to include a "mini explainer" in the pull request description. Something like whatwg/dom#1032. And submit to TAG review. Besides helping to get feedback on the proposal this would also make it easier for chromium to adopt the API more quickly. |
|
@wanderview I simplified the proposed API. There is no more const patterns: Array<[URLPattern, string]>;
const keys = new Map(patterns);
const list = new URLPatternList(patterns.map(r => r[0]));
const res = list.exec(...)
if (res !== null) console.log("Matched", map.get(res.pattern)); |
|
I think @domenic had previously advocated the map approach to me. If this is ergonomic enough for developers, that works for me. |
|
I have added a mini explainer to the PR description. |
|
@lucacasonato What is the status of this? How did your prototyping in deno go? I'm thinking of prototyping this in chromium using re2's set implementation. |
|
I was also wondering if we should rename this to |
|
I filed an intent to prototype for chromium: https://groups.google.com/a/chromium.org/g/blink-dev/c/QrPrveVyFnA/m/PkaPKfJ4AwAJ |
|
@domenic Do you think we should include Edit: Note, the URLPatternSet will be ordered, but not insertion order. It will instead be sorted based on a spec defined algorithm. |
|
Also, I wonder if the Map oriented approach to associating data with entries will make it hard to have a constructor that takes dictionaries instead of full URLPatterns. Not sure if the dictionaries shortcut is worth optimizing for at the cost of added complexity elsewhere. |
|
I've always been -1 on the map-oriented approach, myself. Regarding setlike APIs, I think the main question is whether you want people to introspect and possibly mutate these things after creation. The spec PR here gives an immutable, non-introspectable object, and that seems to accomplish the main use cases. On the other hand, add introspection APIs from setlike (entries, forEach, has, keys, size, values) doesn't seem to hurt. And adding add, clear, and delete also doesn't seem that bad. Note that if you're not using insertion order, you'll need to override the implementation of add(), in order to put the given value in the right place in the set entries. |
Really? I thought you suggested it to me before. I must have been confused. Should we use a maplike API instead? While read-only is all we technically need, web developers have said making it mutable would be nice. Maybe starting read-only would be safest and add mutators later if needed. (For example I'm unsure if putting a duplicate entry in should overwrite or fail.) |
|
I'm sorry, I misunderstood what you meant. I was -1 on changing the API of URLPatternList to be map-oriented. I am +1 on developers using maps separately as a side table. |
|
Ok, I recall you were positive on passing in an array of init dictionaries to the constructor. But that doesn't seem to work with the external map approach. Does that seem ok to you? |
|
To clarify, if you pass in an init dictionary and the constructor makes the URLPattern for you, then you don't have the pattern as a key in your Map. |
|
Yeah, that seems OK to me. I think people who want to associate data that way can use explicit URLPattern objects. |
| }; | ||
|
|
||
| dictionary URLPatternListResult : URLPatternResult { | ||
| URLPattern pattern; |
There was a problem hiding this comment.
I don't know how it works internally but shouldn't it contain the match result as well? Since the point is to pick a pattern to match and it's likely doing some work behind the scenes
There was a problem hiding this comment.
It does — the : URLPatternResult part means this dictionary extends URLPatternResult.
There was a problem hiding this comment.
Aaah i didn’t see it. (Not familiar with this spec language 😂)
| <div algorithm> | ||
| The <dfn constructor for=URLPatternList lt="URLPatternList(patterns)">new URLPatternList(|patterns|)</dfn> constructor steps are: | ||
|
|
||
| 1. [=list/For each=] |pattern| in |patterns|, run the following steps: |
There was a problem hiding this comment.
Doesn't this imply that the pattern list is traversed as an array and is potentially slower? #30 (comment)
There was a problem hiding this comment.
The iteration steps here and elsewhere don’t appear to be observable, so engines can potentially optimize them however they see fit, right? Spec steps usually aim for the simplest and clearest explanation of observable behaviors, not the most efficient ones (because observable behaviors are the only things they can actually specify, further complexity is noise).
|
Does this only do stateless longest matching? Should it be possible to resume pattern matching from a specified pattern ( |
|
@lucacasonato what's the status of this? |
This comment was marked as duplicate.
This comment was marked as duplicate.
…1234) * feat: add url_pattern_list - compiled route-set matching (RFC) ada::url_pattern_list<regex_provider> compiles a set of URLPattern pathname patterns into one dispatch structure (segment trie with per-node witness-byte dispatch, a whole-pathname exact table for fully static routes, and per-shape dispatch tables for parameterized routes), answering "which route matches this pathname, and what are the parameter values?" in tens of nanoseconds instead of a loop of url_pattern::exec calls. This is the URLPatternList use case (whatwg/urlpattern#166), scoped to the pathname component. Design points: - The pattern side reuses ada's own URLPattern machinery: parse_pattern_string + canonicalize_pathname classify each pattern's part list; the static / ":param" / "*" subset is compiled, and patterns needing regexp semantics are matched through the regex provider while participating in the same priority order. - Match priority is specificity order (literal < ":param" < "*", per-segment from the left), insertion order breaking ties - find-my-way/Express-compatible. Whether URLPatternList should use insertion order instead is an open question for the RFC. - Fast-path limits (4096-byte inputs, 24 input segments, 16 pattern segments, 8 captures, 254-entry dispatch tables) are performance gates, not match contracts: inputs and routes beyond them fall back to a sequential matcher with identical semantics, and failed offline table searches demote to linear scans. No input aborts; construction errors return tl::expected (errors::type_error, like the URLPattern constructor). - The matcher is allocation-free and regex-free on the fast path; the NEON segment scan has a portable scalar fallback (ADA_URL_PATTERN_LIST_NO_NEON) and the packed-window compares are endian-safe. Includes a GTest suite (priority/boundary cases, out-of-fast-path inputs, a semantics pin against ada::url_pattern, and a randomized differential test against an independent reference matcher) and a benchmark (benchmarks/urlpattern_list.cpp) comparing a sequential url_pattern::exec loop with url_pattern_list::match over a 101-route REST table: 58.4 us/url vs 54.0 ns/url (~1080x) on Apple M3 Max, answers cross-checked identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: expand url_pattern_list coverage, add fuzzer, guard private API docs Addresses the review feedback on the url_pattern_list RFC: - Tests: coverage-driven expansion of url_pattern_list_tests.cpp (598 -> 1168 lines, 16 -> 32 tests). Adds a data-driven semantics table (~130 pattern/input/capture triplets over every subset syntax feature, subset boundary, and canonicalization interaction, cross-checked row by row against ada::url_pattern::test), targeted tests for every dispatch and demotion rung (witness-plan exhaustion for nodes, shape groups and the exact table, >64-key slot tables, >254-entry linear demotions, 16-bit key overflow, segment-count gate boundaries, probe-order promotion and its dependent-group counterpart, kind-sequence packing caps), group-name and capture-alignment tests including duplicate names across routes, and a per-route url_pattern::test cross-check sampled inside the existing differential test. Local llvm-cov: url_pattern_list.cpp 99.67% lines / 98.57% branches / 100% functions; url_pattern_list-inl.h and url_pattern_list.h 100% on all metrics. - Fuzzing: new fuzz/url_pattern_list.cc (with .options, wired into fuzz/build.sh) drives three strategies from one harness: arbitrary construction over 1..64 derived patterns exercising the full public surface, a differential oracle that rebuilds each route through the sequential helpers (or the compiled pathname component) and aborts on any winner or capture-slice disagreement, and fast-path-gate crossings around the 4096-byte and 24-segment limits. 372k executions over 10 minutes under ASan+UBSan locally: no findings. - Docs: every internal type, function and member in url_pattern_list.h is now guarded with @Private doc comments (27 markers), following character_sets.h/checkers.h conventions; compute_kind_sequence moved out of the public header into the translation unit. Doxygen runs clean over the tree with no warnings for these files. - Benchmarks: the shared URL stream shrinks from 512 to 32 URLs so one iteration of the sequential url_pattern::exec baseline stays well under CodSpeed's per-iteration budget (~2.1 ms locally, was ~34 ms); both benchmarks still iterate the identical stream and the disagreement cross-check is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * build: compile url_pattern_list in its own TU (keeps setter inlining 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 #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> * fix: record ignore_case on url_pattern objects parse_url_pattern_impl forwarded options->ignore_case into the component compile options but never stored it on the url_pattern, so url_pattern::ignore_case() always returned false. parse_url_pattern_list's url_pattern-object overload reads that flag, so store it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: url_pattern_list matches parse_url_pattern; matcher inline, compiler private API: - ada::parse_url_pattern_list<Provider>(span<const string_view>, base_url*, url_pattern_options*) next to parse_url_pattern, plus an overload taking span<const url_pattern<Provider>> that reuses the objects' compiled pathname components and their ignore_case(). url_pattern_list::create is private; the free functions are the entry points. - ignore_case reaches url_pattern_compile_component_options for regexp routes (the provider sees the flag) and the static/":param"/"*" subset compares literals with ASCII case folding (compiled literals folded at creation, the input folded into a stack copy at match time; offsets are unchanged so captures still slice the original). - Regexp routes are tested with regex_match and executed with regex_search for their group values, URLPattern's own test/exec split; the match result carries them as regexp_groups (owned, aligned with group_names) next to the zero-allocation (offset, length) slices of subset routes, with regexp_route telling the two forms apart. Public header and TU split: - include/ada/url_pattern_list.h now holds only the limits, the match result, url_pattern_list and the table records the inline matcher reads; the route-set compiler (route_info, classify_parts, compile_route_set, witness planners, arena packing) moved to src/url_pattern_list_compiler.h, which ada.h does not include. The fuzzer reaches it through the amalgamated ada.cpp, as the other fuzzers do. - The walk (scan_segments, verify_edge, dispatch_static, match_compiled) is ada_really_inline in url_pattern_list-inl.h and inlines into url_pattern_list::match; only the builder stays in the separate TU. The parse_url_pattern_list definitions live in implementation-inl.h, after the defaulted declarations, as parse_url_pattern's does. The unity ada.cpp codegen is unchanged under GCC 14 -O3: all 793 functions, url_aggregator setters included, have identical opcode streams. Match path: - Auxiliary routes (regexp mode, sequential mode) are no longer walked after a fast-path hit: at creation each trie route records the auxiliary routes that both outrank it and could match the same input (literal agreement and segment-count compatibility); after a hit only those are tested. A regexp route whose parts are fixed text and ":name" groups has an exact segment shape, and one with custom groups an anchored literal prefix; match_regexp_shape rejects inputs that cannot fit it before any provider call, on hits and misses alike. - Direct compares up to 16 children (measured against 8: faster on every stream, static hits 28.0 -> 25.9 ns/url); a 256-entry first-byte index at the root (children sorted by first byte, runs of at most 16); projection only for wider nodes. - One arena holds every table (nodes, hash payloads, edges, slots, blob, route records, segment table, aux table, root index), addressed by section offsets. node_record is 24 bytes; the projection payload lives in a separate hash_record only for hashed nodes. - Portable SWAR slash scan (8 bytes per step, exact zero-lane test on x ^ '/'-fill, byte-exact partial tail load, no over-read); keys of 8..16 bytes verify from two whole-word loads inside the segment, shorter keys from a masked load only when 8 bytes are readable and from a byte gather otherwise; memcmp past 16 bytes. eq_bytes, the NEON scanner and endpad are gone. - The whole-pathname exact table and the shape tables are removed: with the root index and direct compares, static hits measured within noise of the exact table and param hits within noise of the shape tables, and the shape tables depended on the exact table's completeness. Empty literal segments ("/users/") are now trie routes. shape_group::{mask, n_static, witness_ids}, n_ids_out and covered_by_static_table go with them. Tests: a counting provider wrapper (create_instance / regex_search / regex_match calls and the ignore_case flag), ignore_case parity with url_pattern, base URL processing, url_pattern objects as input, auxiliary route pruning, SWAR scan sweep, short tails, root index, direct fanout, and match() over exactly sized unterminated buffers. The fuzzer's oracle is now each route's url_pattern pathname component (regex_search included) ranked by the compiler's kind sequences, over exactly sized input buffers. Benchmark (benchmarks/urlpattern_list.cpp, 32-URL stream, ns/url, median of 6 interleaved runs, before -> after): mixed stream 52.4 -> 34.0, static hits 27.7 -> 26.6, param hits 31.9 -> 32.7, table with one "(\d+)" route 114.9 -> 41.9. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: a "*" segment does not match line terminators "*" stands for "(.*)" in the URLPattern regexp, and "." in an ECMAScript regular expression never matches LF or CR, so url_pattern rejects a raw line terminator in the wildcard tail while the compiled matcher accepted it (found by the fuzzer's url_pattern oracle: pattern "/*", input "/\n"). Apply the rule in the trie walk and in the sequential matcher. Canonical pathnames percent-encode both bytes, so only raw inputs are affected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf: direct compares up to 8 children, not 16 Measured on a synthetic node with N static children under a fixed prefix (plus a ":rest" sibling), hit uniformly, with keys of varied length and with keys of one length and prefix, which the length gate never rejects. Three builds: projection from 3 children, direct compares up to 8, direct compares at every fanout. ns/url, median of 3 interleaved runs, written as projection / direct: hits, varied keys N=3 20.5/19.0 N=8 20.6/19.7 N=12 21.1/20.4 N=16 21.0/21.7 N=24 20.7/23.6 hits, same length N=3 16.8/21.4 N=8 16.8/21.8 N=12 16.8/24.9 N=16 16.9/27.1 N=24 16.8/31.1 misses, varied keys N=3 20.6/20.3 N=8 20.7/23.7 N=12 21.2/26.1 N=16 21.1/28.9 misses, same length N=3 17.5/18.9 N=8 17.5/25.5 N=12 17.5/31.0 N=16 17.5/36.5 Direct compares win only for keys of varied length, by about 1 ns, and only up to about 12 children; for keys of one length the projection wins by 4-5 ns at every fanout, and by more on misses. Sixteen had gained 2.5 ns on the PR benchmark table because that stream is skewed toward /api, which makes the direct loop's exit predictable. Eight keeps the common-case win and bounds the loss; on the PR table it is a wash against projection from 3 (static hits 28.0 vs 28.3, ":param" hits 34.9 vs 35.0). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBz2pfstt62q1rGwqbjP9X * perf: check the wildcard tail for line terminators with SWAR, out of line The check added in 5a40de0 walked the captured tail byte by byte on every "*" hit: about 0.3 ns per byte, 1.3 us for a 4 KB pathname and 5-8 ns on a typical one. Now 8 bytes per step with the "some byte below 0x20" test (exact as a yes/no answer) and the exact byte check only for a tail that holds a control byte. Kept out of line on purpose: inlined into the walk, the loop cost the static and ":param" paths, which never run it, about 5 ns through register allocation (27.7 -> 32.4 and 34.9 -> 39.6 ns/url on the PR benchmark); as a call it costs only wildcard hits. Long-pathname benchmark (two-node table, "/files/*" wins, ns/url), the NEON scanner at 840804f vs the SWAR scanner: 32 B, 2 segments 15.4 vs 18.7 128 B, 24 segments 24.1 vs 37.2 128 B, 2 17.3 vs 26.8 1 KB, 24 47 vs 137 1 KB, 2 40 vs 132 4 KB, 24 131 vs 585 4 KB, 2 126 vs 460 PR benchmark, 840804f -> this commit: mixed stream 50.0 -> 34.2, static hits 26.6 -> 27.6, ":param" hits 30.5 -> 35.6, table with one "(\d+)" route 110.9 -> 42.6. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBz2pfstt62q1rGwqbjP9X * perf: keep the NEON segment scan; SWAR stays the portable path The 1k+ pathname benchmark asked for in review: a two-node table where "/files/*" wins, so the scan dominates; the NEON scanner at 840804f against the SWAR loop at a4ab137, ns/url: 32 B, 2 segments 15.4 vs 18.7 128 B, 24 segments 24.1 vs 37.2 128 B, 2 17.3 vs 26.8 1 KB, 24 47 vs 137 1 KB, 2 40 vs 132 4 KB, 24 131 vs 585 4 KB, 2 126 vs 460 NEON is faster at every length on an Apple M3, 1-3 ns on 20-40 byte paths and 3-4x past 1 KB, so 12e6b41 removed it wrongly. Restored under ADA_NEON for inputs of 16 bytes or more: 16 bytes per step, the '/' compare narrowed to one nibble per byte, an overlapped last block so the input is never over-read. The SWAR loop stays as the portable scan and for shorter inputs, and the sweep test exercises both. The wildcard tail check takes the same 16-byte step (a running byte minimum). With this the 32-byte case is back at parity (15.6 vs 15.8 ns/url); the remaining 2x on 4 KB "*" tails is the line-terminator check's second pass over the tail. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBz2pfstt62q1rGwqbjP9X * bench: correct the note on the regexp-route stream A counting provider over the benchmark's 32-URL stream shows the "(\d+)" route is tested once, on an "/api/v1/invoices/.../zz" miss that lands on "/*", where it legitimately could win; the comment claimed std::regex never runs on that stream. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HBz2pfstt62q1rGwqbjP9X * docs: rewrite the URLPattern list section of the README Same shape as the URLPattern section above it: a short introduction, an example with the same provider, and a few plain notes on what is matched, the priority order, the options and the limits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A6djgGCa4kARsjHSipVCbH * fix the builds on Apple clang 15 and MSVC Apple clang 15 does not match a friend declaration that redeclares a constrained function template, so parse_url_pattern_list could not reach the private members of url_pattern_list and the macOS 14 job failed on every use. Declare the two overloads before the class and befriend those specializations instead. Their default arguments stay in implementation.h, which this header always pulls in first, since a function template may not gain default arguments in a later declaration. MSVC's ada_never_inline is __declspec(noinline) with no inline linkage, so the wildcard_tail_ok definition in the header was emitted in every translation unit and the link failed with LNK2005. Move it next to the other detail functions in url_pattern_list.cpp. It was already never inlined, so the call is unchanged. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Closes #30
Mini-explainer:
This adds a new
URLPatternListinterface that is can be used to match manyURLPatterns at once. This new interface can be used to easily create URL routers with a large number of patterns, without compromising on performance.An example of how a user could use this API:
The main motivation for this addition is the performance improvements this can unlock compared to a naive matching algorithm based on the existing
URLPatterninterface. Existing user-land router implementation usingURLPatternslow down linearly with the addition of patterns.URLPatternListis designed to be able to deliver sub linear performance characteristics for routers of any size.TODOs
Preview | Diff