Repository navigation
optimize url::can_parse method - #1106
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (65.33%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1106 +/- ##
==========================================
+ Coverage 59.61% 59.71% +0.10%
==========================================
Files 37 37
Lines 5851 5958 +107
Branches 2851 2907 +56
==========================================
+ Hits 3488 3558 +70
- Misses 586 593 +7
- Partials 1777 1807 +30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by ×3.5
Performance Changes
Comparing Footnotes
|
85b76ee to
9c879b3
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces new fast paths intended to improve URL parsing and validation performance, primarily by short-circuiting the full WHATWG state machine for common absolute special-URL cases.
Changes:
- Added
try_parse_fast()builders forada::urlandada::url_aggregator, and wired them intoparser::parse_url_impl()when no base URL is provided. - Refactored
parser::parse_url_implto remove thestore_valuestemplate parameter and adjusted friend/template declarations accordingly. - Replaced
ada::can_parse()implementation with a custom “zero-allocation” validator plus a fast-path precheck.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/url_aggregator.cpp | Adds url_aggregator::try_parse_fast() single-pass constructor for common absolute special URLs. |
| src/url.cpp | Adds url::try_parse_fast() counterpart populating ada::url fields directly. |
| src/parser.cpp | Uses the new try_parse_fast() methods before the full parser; removes store_values branching. |
| src/implementation.cpp | Reimplements can_parse() with new fast-path and zero-alloc fallback logic. |
| include/ada/url_aggregator.h | Declares url_aggregator::try_parse_fast() and updates parser friend declarations. |
| include/ada/url.h | Declares url::try_parse_fast() and updates parser friend declarations. |
| include/ada/parser.h | Updates parse_url_impl template signature (removes store_values). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1d8a88d to
36c3c28
Compare
36c3c28 to
f8cfddc
Compare
5ea97b9 to
bae2393
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 27 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Exciting! Looks like fuzzers aren't happy yet with: |
…fix stringview usage, update AGENTS.md
… tests for all fuzzer crashes
|
@lemire would you mind reviewing? |
Known IDNA conformance gaps surfaced by the new data (tracked upstream in ada-url/idna, src/ada_idna.cpp is auto-generated): - ContextJ C1 (ZWNJ): xn--ab-j1t - empty punycode label: xn-- - TR46 validity for enclosed alphanumerics: xn--pokxncvks Also reformat src/url_pattern_helpers.cpp for clang-format 22 (the lint CI was bumped from v17 to v22 in ada-url#1106); the existing line wrapping no longer conforms. Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Known IDNA conformance gaps surfaced by the new data (tracked upstream in ada-url/idna, src/ada_idna.cpp is auto-generated): - ContextJ C1 (ZWNJ): xn--ab-j1t - empty punycode label: xn-- - TR46 validity for enclosed alphanumerics: xn--pokxncvks Also reformat src/url_pattern_helpers.cpp for clang-format 22 (the lint CI was bumped from v17 to v22 in ada-url#1106); the existing line wrapping no longer conforms. Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Known IDNA conformance gaps surfaced by the new data (tracked upstream in ada-url/idna, src/ada_idna.cpp is auto-generated): - ContextJ C1 (ZWNJ): xn--ab-j1t - empty punycode label: xn-- - TR46 validity for enclosed alphanumerics: xn--pokxncvks Also reformat src/url_pattern_helpers.cpp for clang-format 22 (the lint CI was bumped from v17 to v22 in ada-url#1106); the existing line wrapping no longer conforms. Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Known IDNA conformance gaps surfaced by the new data (tracked upstream in ada-url/idna, src/ada_idna.cpp is auto-generated): - ContextJ C1 (ZWNJ): xn--ab-j1t - empty punycode label: xn-- - TR46 validity for enclosed alphanumerics: xn--pokxncvks Also reformat src/url_pattern_helpers.cpp for clang-format 22 (the lint CI was bumped from v17 to v22 in ada-url#1106); the existing line wrapping no longer conforms. Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Known IDNA conformance gaps surfaced by the new data (tracked upstream in ada-url/idna, src/ada_idna.cpp is auto-generated): - ContextJ C1 (ZWNJ): xn--ab-j1t - empty punycode label: xn-- - TR46 validity for enclosed alphanumerics: xn--pokxncvks Also reformat src/url_pattern_helpers.cpp for clang-format 22 (the lint CI was bumped from v17 to v22 in ada-url#1106); the existing line wrapping no longer conforms. Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Known IDNA conformance gaps surfaced by the new data (tracked upstream in ada-url/idna, src/ada_idna.cpp is auto-generated): - ContextJ C1 (ZWNJ): xn--ab-j1t - empty punycode label: xn-- - TR46 validity for enclosed alphanumerics: xn--pokxncvks Also reformat src/url_pattern_helpers.cpp for clang-format 22 (the lint CI was bumped from v17 to v22 in ada-url#1106); the existing line wrapping no longer conforms. Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Known IDNA conformance gaps surfaced by the new data (tracked upstream in ada-url/idna, src/ada_idna.cpp is auto-generated): - ContextJ C1 (ZWNJ): xn--ab-j1t - empty punycode label: xn-- - TR46 validity for enclosed alphanumerics: xn--pokxncvks Also reformat src/url_pattern_helpers.cpp for clang-format 22 (the lint CI was bumped from v17 to v22 in ada-url#1106); the existing line wrapping no longer conforms. Signed-off-by: Daijiro Wachi <daijiro.wachi@gmail.com>
Significantly improves the performance of can_parse method.
Before merging, I need to do some cleaning and move methods to correct places, add documentation etc.