Skip to content

optimize url::can_parse method#1106

Open
anonrig wants to merge 31 commits intomainfrom
yagiz/optimize-canparse
Open

optimize url::can_parse method#1106
anonrig wants to merge 31 commits intomainfrom
yagiz/optimize-canparse

Conversation

@anonrig
Copy link
Copy Markdown
Member

@anonrig anonrig commented Mar 29, 2026

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
Copy link
Copy Markdown

codecov bot commented Mar 29, 2026

Codecov Report

❌ Patch coverage is 63.75839% with 54 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.64%. Comparing base (a3cbb2c) to head (5376065).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/implementation.cpp 66.34% 4 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 ⚠️
src/parser.cpp 60.00% 1 Missing and 1 partial ⚠️
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 ⚠️

❌ Your patch check has failed because the patch coverage (63.75%) 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.64%   +0.03%     
==========================================
  Files          37       37              
  Lines        5851     5957     +106     
  Branches     2851     2907      +56     
==========================================
+ Hits         3488     3553      +65     
- Misses        586      595       +9     
- Partials     1777     1809      +32     

☔ 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-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 29, 2026

Merging this PR will improve performance by ×3.4

⚡ 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 +34.18%
BBC_BasicBench_AdaURL_CanParse 12 µs 3.5 µs ×3.4
Bench_IPv4_NonDecimal_Aggregator 5.4 ms 5.1 ms +5.83%
BenchData_BasicBench_AdaURL_CanParse 66.6 ms 21.7 ms ×3.1

Comparing yagiz/optimize-canparse (5376065) with main (ed4fa6c)

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
Copy link
Copy Markdown

Copilot AI left a comment

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.

@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
Copy link
Copy Markdown

Copilot AI left a comment

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.

@CarlosEduR
Copy link
Copy Markdown
Member

Exciting!

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

anonrig added 20 commits March 30, 2026 09:49
@anonrig
Copy link
Copy Markdown
Member Author

anonrig commented Mar 31, 2026

@lemire would you mind reviewing?

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