Skip to content

perf: simple-absolute href cache and string pool for ada::url - #1201

Open
anonrig wants to merge 2 commits into
mainfrom
perf/url-href-cache-pool
Open

perf: simple-absolute href cache and string pool for ada::url#1201
anonrig wants to merge 2 commits into
mainfrom
perf/url-href-cache-pool

Conversation

@anonrig

@anonrig anonrig commented Jul 31, 2026

Copy link
Copy Markdown
Member

Summary

Further speed up ada::url parse + get_href on the simple-absolute path:

  1. Href cache — store the prebuilt href in non_special_scheme (unused for special schemes); get_href() returns a copy. Clear the cache on setters. Fill host/path/query/hash at parse so getters stay normal.
  2. Bounded string pool — single thread-local spare (string_pool) reuses href-buffer capacity across parse/destroy (capacity only retained in [24, 1024]).

Not included: freelist on url_aggregator (previously regressed CodSpeed IPv4 aggregator).

Self-contained for main

This PR includes the widened simple-absolute scanner so it builds against current main without using another PR branch as base (not stacked).

  • If the simple-absolute-only PR merges first, rebase this branch and drop the overlapping parse-only delta.
  • Prefer merge order: simple-absolute PR (optional) then this PR; get-href micro-opts PR can land anytime.

Scope

  • string_pool.{h,cpp}, url / url-inl / url.cpp, parser.cpp, tests
  • Inline url destructor only (recycles pool); url_aggregator stays defaulted

Test plan

  • Full CI
  • CodSpeed: confirm no IPv4 aggregator regression

On the simple-absolute path, store a prebuilt href in non_special_scheme
and return it from get_href. Clear the cache on setters. Use a single
thread-local spare (string_pool) to reuse that buffer capacity.

Self-contained for main (includes widened simple-absolute scanner).
Does not pool url_aggregator buffers (avoids IPv4 CodSpeed regressions).
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 81.73077% with 38 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.25%. Comparing base (d154358) to head (6a4fcd9).

Files with missing lines Patch % Lines
src/parser.cpp 82.70% 5 Missing and 18 partials ⚠️
include/ada/url-inl.h 74.50% 0 Missing and 13 partials ⚠️
src/string_pool.cpp 87.50% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1201      +/-   ##
==========================================
+ Coverage   61.06%   61.25%   +0.19%     
==========================================
  Files          38       39       +1     
  Lines        6939     7004      +65     
  Branches     3231     3260      +29     
==========================================
+ Hits         4237     4290      +53     
- Misses        749      752       +3     
- Partials     1953     1962       +9     

☔ 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 9.56%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 7 improved benchmarks
✅ 22 untouched benchmarks
⏩ 4 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
url_search_params_AdaURL 126.4 µs 109 µs +15.93%
BenchData_BasicBench_AdaURL_aggregator_href 66.8 ms 57.9 ms +15.41%
Bench_DNS_Aggregator 67.6 ms 58.7 ms +15.23%
BenchData_BasicBench_AdaURL_href 98.1 ms 90.3 ms +8.64%
Bench_IPv6_Aggregator 4.1 ms 3.9 ms +4.57%
Bench_IPv6_AdaURL 3.3 ms 3.1 ms +4.37%
Bench_BasicBench_AdaURL_aggregator_href 22.1 µs 21.3 µs +3.64%

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/url-href-cache-pool (6a4fcd9) 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.

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 optimizes the common “simple-absolute” HTTP(S) parsing path and ada::url::get_href() by caching a prebuilt href string and reusing string capacity via a bounded, thread-local string spare, aiming to improve hot-path performance without changing observable URL semantics.

Changes:

  • Add a simple-absolute href cache stored in url::non_special_scheme, with cache invalidation on URL mutations and a faster get_href() fast-path.
  • Introduce ada::string_pool (single thread-local spare string) to reuse std::string capacity across parse/destroy within bounded limits.
  • Expand/adjust the simple-absolute parser fast path (host/path scanning and normalization checks) and add regression tests for can_parse and simple-absolute behaviors.

Reviewed changes

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

Show a summary per file
File Description
tests/basic_tests.cpp Adds regressions for can_parse consistency with credentials and expands simple-absolute fast-path test cases.
src/url.cpp Clears the simple href cache on relevant setters/mutations.
src/string_pool.cpp Implements the thread-local spare string reuse logic.
src/parser.cpp Implements the new simple-absolute scanning/validation logic and populates the href cache + fields on the fast path.
src/implementation.cpp Documents why parse should not redundantly call the fast-path validator.
src/ada.cpp Adds string_pool.cpp into the single translation unit build.
include/ada/url.h Declares the inline destructor and cache helper methods.
include/ada/url-inl.h Defines destructor recycling behavior and adds href-cache-aware get_href()/get_href_size() plus scheme-copy adjustments.
include/ada/string_pool.h Declares the bounded string pool API and capacity bounds.

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

Comment thread include/ada/url-inl.h
Comment on lines 196 to +200
constexpr void url::copy_scheme(ada::url&& u) {
non_special_scheme = u.non_special_scheme;
type = u.type;
// non_special_scheme holds the scheme name only for non-special URLs. For
// special URLs it may hold a simple-absolute href cache - never copy that.
if (u.type == ada::scheme::type::NOT_SPECIAL) {
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 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

include/ada/url.h:69

  • The comment says the destructor recycles both path and the href-cache, but the new inline destructor only recycles non_special_scheme (the href-cache). Please update the comment so it matches the actual behavior.
  // Inline (see url-inl.h): recycles path / href-cache capacity into a bounded
  // thread-local freelist. Kept inline to match main's defaulted dtor ABI.

Comment thread include/ada/url.h
Comment on lines +68 to +70
// Inline (see url-inl.h): recycles path / href-cache capacity into a bounded
// thread-local freelist. Kept inline to match main's defaulted dtor ABI.
~url() override;
@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