bitcoin
bitcoin copied to clipboard
bitcoin
bench: enable wallet creation benchmarks on all platforms
Simple fix for #29816.
Instead of re-using a single directory to create a new wallet on each round, use a different one. The benchmarking framework runs on top of the test framework, which cleans up the entire directory after completion anyway.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage
For detailed information about the code coverage, see the test coverage report.
Reviews
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | maflcko, hebasto, kevkevinpal |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
No conflicts as of last run.
The native Windows CI (https://github.com/bitcoin/bitcoin/actions/runs/9112695910/job/25052498545?pr=30122#step:24:64) is failing with:
Run src\bench_bitcoin.exe -sanity-check
Running with -sanity-check option, output is being suppressed as benchmark results will be useless.
Error: Process completed with exit code 1.
It'd be good if the commit message could explain why this is being done. It seems like the current approach is just to try and refactor away the Windows bug, even though it might not have been properly identified (prior to https://github.com/bitcoin/bitcoin/issues/29816#issuecomment-2115302513). However now that the bug might have been found, it'd be good to at least document what it is, and if possible, fix/workaround it in a more targetted way, rather than refactor code that is currently perfectly fine for all other platforms.
It'd be good if the commit message could explain why this is being done. It seems like the current approach is just to try and refactor away the Windows bug, even though it might not have been properly identified (prior to #29816 (comment)). However now that the bug might have been found, it'd be good to at least document what it is, and if possible, fix/workaround it in a more targetted way, rather than refactor code that is currently perfectly fine for all other platforms.
Sure with a small nuance. This code is working fine for all other platforms but that does not mean the code is perfect. When I wrote this benchmark, I was probably more concerned about the number of files the benchmark could open in parallel than the delays/inaccuracies introduced by the, OS dependent, directory removal line. Essentially, we shouldn't be removing a directory within the benchmarked code scope. But yeah, np on splitting this in two well described commits.
