ci: drop -priority-level from bench in win cross CI #32288

1. 
   fanquake commented at 3:32 pm on April 16, 2025: member

   So there’s at least one CI sanity checking all benchmarks.

   Related to #32277.

2. 
   DrahtBot commented at 3:32 pm on April 16, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32288.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	l0rinc, hebasto, mabu44

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #32219 (DRAFT: test: enabling wallet migration functional test on windows by m3dwards)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot added the label Tests on Apr 16, 2025
4. 
   l0rinc commented at 3:47 pm on April 16, 2025: contributor

   utACK 3f602f056001dc36657f01a938f62ea722f78854 - was thinking of pushing something similar
5. ci: drop -priority-level from bench in win cross CI

   So there's at least one CI sanity checking all benchmarks.
   
   Related to #32277.

   27f11217ca
6. fanquake force-pushed on Apr 17, 2025
7. in .github/workflows/ci.yml:361 in 27f11217ca

   357@@ -358,7 +358,7 @@ jobs:
   358           ./src/univalue/unitester.exe
   359 
   360       - name: Run benchmarks
   361-        run: ./bin/bench_bitcoin.exe -sanity-check -priority-level=high
   362+        run: ./bin/bench_bitcoin.exe -sanity-check
   

   ---

   ---

   

   hebasto commented at 9:25 am on April 17, 2025:

   Why not drop -priority-level=high here as well:https://github.com/bitcoin/bitcoin/blob/7a3afe6787bc36fd98684eac020f9abdbfae6f0a/src/bench/CMakeLists.txt#L82-L84 ?

   Will it have a significant impact on execution time?

   ---

   

   fanquake commented at 9:27 am on April 17, 2025:

   I’m just changing CI, so that we catch regressions. Someone could follow up with changing this for all developers & users.

   ---

   

   hebasto commented at 9:44 am on April 17, 2025:

   But this introduces another inconsistency between Windows and other platforms. Initially, run: ./bin/bench_bitcoin.exe -sanity-check -priority-level=high was written to mimic the bench_sanity_check_high_priority test task behaviour.

   ---

   

   fanquake commented at 9:45 am on April 17, 2025:

   Can you explain your issue with running more tests in this CI?

   ---

   

   hebasto commented at 9:54 am on April 17, 2025:

   If it only takes a couple of extra seconds, why run a different set of tests for each platform?

   Windows should, to the best of our ability, be treated the same as any other platform.

   ---

   

   fanquake commented at 10:06 am on April 17, 2025:

   If it only takes a couple of extra seconds,

   I haven’t measured, for all users, developers and CIs. The point of this change is to immediately start catching regressions in at least one CI.

   ---

   

   hebasto commented at 10:15 am on April 17, 2025:

   The point of this change is to immediately start catching regressions in at least one CI.

   Ok. Resolved.

   ---

   

   maflcko commented at 10:18 am on April 17, 2025:

   The suggestion sounds reasonable. I’d be happy to review a pull doing it.
8. 
   l0rinc commented at 9:39 am on April 17, 2025: contributor

   utACK 27f11217ca63e0f8f78f14db139150052dcd9962
9. hebasto approved
10. 
    hebasto commented at 10:13 am on April 17, 2025: member

    ACK 27f11217ca63e0f8f78f14db139150052dcd9962.
11. 
    mabu44 commented at 10:33 am on April 17, 2025: none

    utACK 27f11217ca63e0f8f78f14db139150052dcd9962
12. fanquake merged this on Apr 17, 2025
13. fanquake closed this on Apr 17, 2025

    ---

14. fanquake deleted the branch on Apr 17, 2025
15. 
    l0rinc commented at 4:36 pm on April 17, 2025: contributor

    Seems related: https://github.com/l0rinc/bitcoin/actions/runs/14519683955/job/40737543737?pr=8

    Running with -sanity-check option, output is being suppressed as benchmark results will be useless. terminate called after throwing an instance of ‘std::filesystem::__cxx11::filesystem_error’ what(): filesystem error: cannot remove all: The process cannot access the file because it is being used by another process [C:\Users\RUNNER1\AppData\Local\Temp\test_common bitcoin\WalletMigration\af1d7c4664d64d17a328] [C:\Users\RUNNER1\AppData\Local\Temp\test_common bitcoin\WalletMigration\af1d7c4664d64d17a328\regtest\wallet.dat]

Contributors
fanquake DrahtBot l0rinc hebasto maflcko mabu44

Labels
Tests