Skip to content

perf: widen simple-absolute http(s) parse fast path - #1199

Open
anonrig wants to merge 2 commits into
mainfrom
perf/simple-absolute-parse
Open

perf: widen simple-absolute http(s) parse fast path#1199
anonrig wants to merge 2 commits into
mainfrom
perf/simple-absolute-parse

Conversation

@anonrig

@anonrig anonrig commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

Widen the existing try_parse_simple_absolute hot path for absolute http/https (no base URL):

  • Bulk host / rest validation (NEON on Apple Silicon where available)
  • memchr locate for ? / #
  • Fail-closed host checks (is_ipv4, punycode) and path . / .. / %2e rejection
  • Cheap digit / [ peek so pure IPv4/IPv6 inputs never enter the fast path

On success, fill url components and the url_aggregator buffer directly (no freelist, no href cache).

Scope

  • src/parser.cpp, tests/basic_tests.cpp, small comment in src/implementation.cpp
  • No string_pool, no url href cache, no public API signature changes

Independence

Targets main only. Independent of the get-href and href-cache PRs (not stacked).

Test plan

  • basic_tests simple-absolute cases
  • Full CI
  • Optional: Release benchdata filter for aggregator/url href

Strengthen try_parse_simple_absolute with bulk host/rest scanning (NEON
where available), memchr delimiter locate, fail-closed host/path checks,
and a cheap digit/IPv6 peek so pure IP inputs skip the fast path.

Fill url components and the aggregator buffer directly on success. No
string pool or href cache in this change.
Copilot AI review requested due to automatic review settings July 31, 2026 17:03
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.05085% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.06%. Comparing base (d154358) to head (68e3d77).

Files with missing lines Patch % Lines
src/parser.cpp 83.05% 5 Missing and 15 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1199   +/-   ##
=======================================
  Coverage   61.06%   61.06%           
=======================================
  Files          38       38           
  Lines        6939     6943    +4     
  Branches     3231     3239    +8     
=======================================
+ Hits         4237     4240    +3     
- Misses        749      752    +3     
+ Partials     1953     1951    -2     

☔ View full report in Codecov by Harness.
📢 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

codspeed-hq Bot commented Jul 31, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 10.01%

⚡ 10 improved benchmarks
✅ 19 untouched benchmarks
⏩ 4 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
Bench_DNS_AdaURL 83.7 ms 70.7 ms +18.34%
url_search_params_AdaURL 126.4 µs 109 µs +15.93%
BenchData_BasicBench_AdaURL_aggregator_href 66.8 ms 57.9 ms +15.41%
BenchData_BasicBench_AdaURL_href 98.1 ms 85.1 ms +15.27%
Bench_DNS_Aggregator 67.6 ms 58.7 ms +15.18%
Bench_IPv6_AdaURL 3.3 ms 3.1 ms +5.31%
Bench_BasicBench_AdaURL_href 27 µs 25.8 µs +4.7%
Bench_IPv6_Aggregator 4.1 ms 3.9 ms +4.54%
BBC_BasicBench_AdaURL_href 18.3 µs 17.6 µs +3.93%
Bench_BasicBench_AdaURL_aggregator_href 22.1 µs 21.4 µs +3.08%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing perf/simple-absolute-parse (68e3d77) with main (d154358)

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 commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

clangcl (Debug) failed with gtest discovery timeout (5s) on basic_tests / wpt_url_tests after link — empty output, no assert. Release clangcl and MSVC Debug passed.

This looks like Windows Debug discovery flakiness under load rather than a hang in try_parse_simple_absolute (no global init; NEON is offline on that job). Re-run if it persists.

CodSpeed for this PR is green with improvements and no IPv4 aggregator regressions.

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 widens Ada’s existing try_parse_simple_absolute fast path for absolute http/https URLs (no base URL) to validate and materialize components more efficiently, aiming to reduce overhead in common “already-normalized” inputs while fail-closing to the full state machine for edge cases (credentials, IP literals, punycode, and dot-segment normalization triggers).

Changes:

  • Adds bulk host scanning and bulk “rest” validation (NEON where available), plus memchr-based ?/# location to reduce per-byte branching in the hot path.
  • Tightens fast-path eligibility and fallthrough conditions (credentials/port rejection, IPv4/punycode checks, dot-segment / %2e path normalization detection).
  • Extends test coverage for fast-path fallthrough and correctness around credentials and IPv4-like host edge cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/basic_tests.cpp Adds regressions/coverage for credentialed URLs and additional simple-absolute fast-path edge cases.
src/parser.cpp Implements widened simple-absolute http(s) fast path with bulk scanning/validation and direct component/buffer filling.
src/implementation.cpp Documents why try_parse should not be redundantly called from parse() due to performance regressions on non-simple inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/parser.cpp
Comment on lines +72 to +74
// Lowercase domain host chars only (no uppercase). 1 = continue, 0 = stop.
// Used for the common already-normalized path (~99% of web URLs).
constexpr std::array<uint8_t, 256> k_host_clean = []() consteval {
CMake 4.x POST_BUILD discovery under clang-cl Debug sometimes yields empty
JSON and fails the build. PRE_TEST defers listing to ctest and is reliable
for our static Windows test links. Also set DISCOVERY_TIMEOUT explicitly.
Copilot AI review requested due to automatic review settings July 31, 2026 17:18

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 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/parser.cpp:53

  • k_rest is now unused in this file (the fast path uses rest_is_clean/k_rest_ok instead). Leaving an unused constexpr table at namespace scope can trigger -Wunused-const-variable warnings in some builds; either remove it or mark it [[maybe_unused]].
// 0 = ok, 1 = ?/#, 2 = reject. '%' is allowed (already-encoded); "%2e" is
// checked later via path_needs_norm so serialization stays exact.
constexpr std::array<uint8_t, 256> k_rest = []() consteval {
  std::array<uint8_t, 256> t{};

Comment thread src/parser.cpp
Comment on lines +134 to +138
const uint8x8_t nib = vshrn_n_u16(vreinterpretq_u16_u8(bad), 4);
const uint64_t bits = vget_lane_u64(vreinterpret_u64_u8(nib), 0);
if (bits != 0) {
return i + (size_t(__builtin_ctzll(bits)) >> 2);
}
@anonrig
anonrig requested a review from lemire July 31, 2026 19:17
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