Skip to content

Conversation

@the-moisrex
Copy link
Contributor

This makes some ifs if constexprs even though it has no affect in the benchmarks, but it's better to be explicit.

Also removed a useless enum.

@the-moisrex
Copy link
Contributor Author

Also, I'm getting these warnings:

/ada-url/benchmarks/benchmark_template.cpp:27:9: warning: ‘++’ expression of ‘volatile’-qualified type is deprecated [-Wvolatile]
   27 |         success++;
      |         ^~~~~~~
/ada-url/benchmarks/benchmark_template.cpp:42:11: warning: ‘++’ expression of ‘volatile’-qualified type is deprecated [-Wvolatile]
   42 |           success++;
      |           ^~~~~~~
/ada-url/benchmarks/benchmark_template.cpp: In function ‘void Bench_BasicBench_AdaURL_CanParse(benchmark::State&)’:

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 10, 2025

CodSpeed Performance Report

Merging #1037 will improve performances by 17.56%

Comparing the-moisrex:benchmark-constexpr (d3867e2) with main (236e519)

Summary

⚡ 1 improvement
✅ 18 untouched
⏩ 3 skipped1

Benchmarks breakdown

Benchmark BASE HEAD Change
BasicBench_whatwg 31.7 µs 27 µs +17.56%

Footnotes

  1. 3 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
Copy link
Member

anonrig commented Dec 11, 2025

Also, I'm getting these warnings:


/ada-url/benchmarks/benchmark_template.cpp:27:9: warning: ‘++’ expression of ‘volatile’-qualified type is deprecated [-Wvolatile]

   27 |         success++;

      |         ^~~~~~~

/ada-url/benchmarks/benchmark_template.cpp:42:11: warning: ‘++’ expression of ‘volatile’-qualified type is deprecated [-Wvolatile]

   42 |           success++;

      |           ^~~~~~~

/ada-url/benchmarks/benchmark_template.cpp: In function ‘void Bench_BasicBench_AdaURL_CanParse(benchmark::State&)’:



this should be fixed too... no rush

@anonrig anonrig merged commit 52be58d into ada-url:main Dec 11, 2025
50 of 51 checks passed
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