Skip to content

optimize url::can_parse method - #1106

Merged
anonrig merged 33 commits into
mainfrom
yagiz/optimize-canparse
Apr 5, 2026
Merged

anonrig merged 33 commits into
mainfrom
yagiz/optimize-canparse

Conversation

@anonrig

@anonrig anonrig commented Mar 29, 2026

Copy link
Copy Markdown
Member

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.

@codecov

codecov Bot commented Mar 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.33333% with 52 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.71%. Comparing base (a3cbb2c) to head (d287dff).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/implementation.cpp 67.30% 3 Missing and 31 partials ⚠️
src/url_aggregator.cpp 0.00% 7 Missing and 1 partial ⚠️
src/url.cpp 20.00% 0 Missing and 4 partials ⚠️
include/ada/url_aggregator-inl.h 0.00% 0 Missing and 2 partials ⚠️
include/ada/url.h 75.00% 0 Missing and 1 partial ⚠️
include/ada/url_aggregator.h 75.00% 0 Missing and 1 partial ⚠️
include/ada/url_search_params-inl.h 87.50% 0 Missing and 1 partial ⚠️
src/parser.cpp 83.33% 0 Missing and 1 partial ⚠️

❌ 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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed

codspeed Bot commented Mar 29, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by ×3.5

⚡ 4 improved benchmarks
✅ 23 untouched benchmarks
⏩ 4 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ Bench_BasicBench_AdaURL_CanParse 18.1 µs 13.5 µs +33.33%
⚡ Bench_IPv4_NonDecimal_Aggregator 5.4 ms 5.1 ms +5.83%
⚡ BenchData_BasicBench_AdaURL_CanParse 66.6 ms 21.7 ms ×3.1
⚡ BBC_BasicBench_AdaURL_CanParse 12 µs 3.4 µs ×3.5

Comparing yagiz/optimize-canparse (d287dff) with main (5bb6647)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@anonrig
anonrig force-pushed the yagiz/optimize-canparse branch from 85b76ee to 9c879b3 Compare March 29, 2026 17:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 for ada::url and ada::url_aggregator, and wired them into parser::parse_url_impl() when no base URL is provided.
  • Refactored parser::parse_url_impl to remove the store_values template 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.

Comment thread src/implementation.cpp Outdated
Comment thread src/implementation.cpp
Comment thread src/implementation.cpp Outdated
Comment thread src/implementation.cpp
@anonrig
anonrig force-pushed the yagiz/optimize-canparse branch 9 times, most recently from 1d8a88d to 36c3c28 Compare March 29, 2026 18:24
@anonrig
anonrig force-pushed the yagiz/optimize-canparse branch from 5ea97b9 to bae2393 Compare March 29, 2026 18:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread include/ada/url_aggregator.h Outdated
Comment thread include/ada/url.h Outdated
Comment thread include/ada/url.h Outdated
Comment thread src/implementation.cpp
Comment thread .github/workflows/abi-check.yml Outdated
Comment thread src/checkers.cpp Outdated
Comment thread src/checkers.cpp Outdated
@CarlosEduR

Copy link
Copy Markdown
Member

Exciting!

Looks like fuzzers aren't happy yet with: ws:.

Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
@anonrig

anonrig commented Mar 31, 2026

Copy link
Copy Markdown
Member Author

@lemire would you mind reviewing?

@anonrig
anonrig merged commit 95895d6 into main Apr 5, 2026
54 of 55 checks passed
@anonrig
anonrig deleted the yagiz/optimize-canparse branch April 5, 2026 17:40
watilde added a commit to watilde/ada that referenced this pull request Jun 28, 2026
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>
watilde added a commit to watilde/ada that referenced this pull request Jun 28, 2026
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>
watilde added a commit to watilde/ada that referenced this pull request Jun 28, 2026
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>
watilde added a commit to watilde/ada that referenced this pull request Jun 28, 2026
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>
watilde added a commit to watilde/ada that referenced this pull request Jun 28, 2026
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>
watilde added a commit to watilde/ada that referenced this pull request Jun 28, 2026
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>
watilde added a commit to watilde/ada that referenced this pull request Jun 28, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants