URLPattern: replace std::string with std::string_view

[CarlosEduR]

I noticed that WebKit already uses a StringView for these parameters.

[codspeed-hq]

CodSpeed Performance Report

Merging #1032 will degrade performances by 99.98%

_{Comparing CarlosEduR:perf/urlpattern-string-view (98ce082) with main (4f1908e)}

Summary

❌ 2 regressions
✅ 11 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
❌	BasicBench_AdaURL_CanParse	11.9 µs	18.6 µs	-35.95%
❌	BasicBench_AdaURL_aggregator_href	16.3 µs	94,934.4 µs	-99.98%

[CarlosEduR]

instructions/url went from 238.859k to 229.012k

before:

---------------------------------------------------------------------------------------------
Benchmark                                   Time             CPU   Iterations UserCounters...
---------------------------------------------------------------------------------------------
BasicBench_AdaURL_URLPattern_Parse     178485 ns       178469 ns         3896 GHz=4.20126 cycle/byte=1.82993k cycles/url=91.2676k instructions/byte=4.78915k instructions/cycle=2.61713 instructions/ns=10.9952 instructions/url=238.859k ns/url=21.7239k speed=2.23568M/s time/byte=447.291ns time/url=22.3086us url/s=44.8257k/s

after:

---------------------------------------------------------------------------------------------
Benchmark                                   Time             CPU   Iterations UserCounters...
---------------------------------------------------------------------------------------------
BasicBench_AdaURL_URLPattern_Parse     175906 ns       175901 ns         4003 GHz=4.10183 cycle/byte=1.74331k cycles/url=86.9475k instructions/byte=4.59172k instructions/cycle=2.63391 instructions/ns=10.8039 instructions/url=229.012k ns/url=21.1973k speed=2.26832M/s time/byte=440.855ns time/url=21.9876us url/s=45.4801k/s

[lemire]

@anonrig Do you understand the codspeed warning ? It seems to say that this PR leads to a drastic performance drop in some tests. How is that possible?

[anonrig]

@anonrig Do you understand the codspeed warning ? It seems to say that this PR leads to a drastic performance drop in some tests. How is that possible?

I think it's just flaky. cc @art049

[GuillaumeLagrange]

Hello! @CarlosEduR @lemire @anonrig

Looking into this, I see that for some reason, three different pids upload results for the benchmarks
benchmarks/benchmark_template.cpp::BasicBench_AdaURL_href
benchmarks/benchmark_template.cpp::BasicBench_AdaURL_aggregator_href
benchmarks/benchmark_template.cpp::BasicBench_AdaURL_CanParse

This is definitely undefined behavior for CodSpeed, and we'll fix it to have it be an explicit error.

However, if one of you has an idea as to why the benchmarks are executed thrice when running cmake --build build --target run_all_benchmarks, this should fix the problem here.

[CarlosEduR]

Thanks for checking that, @GuillaumeLagrange!

However, if one of you has an idea as to why the benchmarks are executed thrice when running cmake --build build --target run_all_benchmarks, this should fix the problem here.

I believe it's related to the way we have our benchmarks configured...

The bench benchmark runs these three benchmarks (BasicBench_AdaURL_href, BasicBench_AdaURL_aggregator_href and BasicBench_AdaURL_CanParse). The benchdata and bbc_bench benchmarks execute the same functions but with different input data.

Here is the PR integrating with Codspeed, in case you wanna take a look: https://github.com/ada-url/ada/pull/1031/files

@anonrig I think it's fine to run only benchdata from these, what do you think? I can open a PR updating it shortly.

[anonrig]

@CarlosEduR they all benchmark different things. can't we just change the name so they don't clash on codspeed?

[CarlosEduR]

@CarlosEduR they all benchmark different things. can't we just change the name so they don't clash on codspeed?

Yeah, I think this fixes the issue too

[GuillaumeLagrange]

Google benchmark has features for this kind of usage

https://github.com/google/benchmark/blob/main/docs/user_guide.md#templated-benchmarks-that-take-arguments

If you have any question/issues about how each benchmark is identified with codspeed's google benchmark layer, do not hesitate to create a dedicated issue on https://github.com/CodSpeedHQ/codspeed-cpp
You should also be able to manually name the benchmarks.