Skip to content

Improve consistency in optimized can_parse - #1119

Merged
anonrig merged 4 commits into
ada-url:yagiz/optimize-canparsefrom
CarlosEduR:csousa-fix-canparse-consistency
Apr 3, 2026
Merged

anonrig merged 4 commits into
ada-url:yagiz/optimize-canparsefrom
CarlosEduR:csousa-fix-canparse-consistency

Conversation

@CarlosEduR

Copy link
Copy Markdown
Member

No description provided.

@CarlosEduR

Copy link
Copy Markdown
Member Author

Regression test failed as expected, pushing the fix now...

[ RUN      ] basic_tests.can_parse_consistency_special_chars_in_authority
/src/tests/basic_tests.cpp:400: Failure
Expected equality of these values:
  cp
    Which is: false
  agg.has_value()
    Which is: true
can_parse/parse<url_aggregator> mismatch for: ws:// @@@@@@@@@@@@@@@@@@@@@@@@:@@@@�@@@@@@@@@@@@5

@codecov

codecov Bot commented Apr 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.72%. Comparing base (77c6d5f) to head (b5d2fad).
⚠️ Report is 1 commits behind head on yagiz/optimize-canparse.

Additional details and impacted files
@@                     Coverage Diff                     @@
##           yagiz/optimize-canparse    #1119      +/-   ##
===========================================================
+ Coverage                    59.66%   59.72%   +0.06%     
===========================================================
  Files                           37       37              
  Lines                         5958     5957       -1     
  Branches                      2907     2906       -1     
===========================================================
+ Hits                          3555     3558       +3     
+ Misses                         594      593       -1     
+ Partials                      1809     1806       -3     

☔ 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.

@anonrig

anonrig commented Apr 3, 2026

Copy link
Copy Markdown
Member

Can you run the formatter?

@CarlosEduR

Copy link
Copy Markdown
Member Author
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Benchmark                                     Time             CPU   Iterations        GHz cycle/byte cycles/url instructions/byte instructions/cycle instructions/ns instructions/url     ns/url      speed  time/byte   time/url      url/s
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BenchData_BasicBench_AdaURL_CanParse    3910503 ns      3910147 ns          181    4.28923    1.82396    158.428           6.41695            3.51814         15.0901          557.371    36.9361 2.22193G/s  450.058ps  39.0917ns 25.5809M/s

@CarlosEduR
CarlosEduR marked this pull request as ready for review April 3, 2026 18:54
@anonrig
anonrig merged commit d287dff into ada-url:yagiz/optimize-canparse Apr 3, 2026
93 of 97 checks passed
anonrig added a commit that referenced this pull request Apr 5, 2026
* optimize url::can_parse method

* update clang-tools to 22

* create AGENTS.md

* remove unused methods

* update comments & abi-check

* bump SOVERSION to 5 for intentional ABI break

* fix clang-tidy-22 warnings: noexcept-escape, unchecked-optional-access, throwing-static-init

* address fuzzing issues

* fix throwing-static-init false positive and add clang-tidy to run-clangcldocker.sh

* fix docker clang-tidy: generate compile_commands.json on host, run tidy in container

* fix gen_compile_commands: drop -stdlib=libc++ when using host GCC

* wipe stale cmake cache before gen_compile_commands to drop old CXX_FLAGS

* fix docker clang-tidy: install cmake+ninja in container, use clang++-22 to match CI exactly

* wipe build-clang-tidy before docker cmake to avoid generator mismatch

* install clang-22 and libc++-22-dev in docker tidy container

* reduce apt-get verbosity with -qq flag

* add git to docker deps for CPM to clone gtest

* suppress apt/docker verbosity, fix SSL certs, cache CPM downloads on host

* exclude vendored gtest from clang-tidy and update ExcludeHeaderFilterRegex

* scope clang-tidy to src/ only, fix git safe.directory, simplify docker setup

* fix all clang-tidy issues: scope to ada.cpp, NOLINT false positives, fix stringview usage, update AGENTS.md

* remove .cpm-cache from repo, add to .gitignore

* add regression tests for extra-slash fuzzer crashes (ws:///..., ws://////5...)

* fix % in host: return nullopt to defer to full parser; add regression tests for all fuzzer crashes

* fix port leading-zeros: strip zeros before pl>5 check; add regression tests

* fix IPv4 fast path bypassing port validation; add regression test

* Update CMakeLists.txt

* Update CMakeLists.txt

* Update CMakeLists.txt

* add shortcuts for can_parse slow path

* optimize even further (#1111)

* Fix error in optimized can_parse (#1118)

* Improve consistency in optimized can_parse (#1119)

---------

Co-authored-by: Carlos Sousa <40635471+CarlosEduR@users.noreply.github.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.

2 participants