Skip to content

bench: Fix benchmarks filters #18013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
maflcko merged 1 commit into from
Jan 28, 2020
Merged

bench: Fix benchmarks filters #18013

maflcko merged 1 commit into from
Jan 28, 2020

Conversation

@elichai
Copy link
Contributor

@elichai elichai commented Jan 28, 2020

The bug was introduced in #17781
before this fix ./src/bench/bench_bitcoin -filter=* will fail with:

# Benchmark, evals, iterations, total, min, max, median
bench_bitcoin: bench/bench.cpp:119: static void benchmark::BenchRunner::RunAll(benchmark::Printer&, uint64_t, double, const string&, bool): Assertion `g_testing_setup == nullptr' failed.
Aborted (core dumped)

@maflcko
Copy link
Member

maflcko commented Jan 28, 2020

ACK 0dae5a5

Thanks for the fix

@maflcko maflcko added the Tests label Jan 28, 2020
@instagibbs
Copy link
Member

instagibbs commented Jan 28, 2020

I wonder if there might be a good place to add a really basic CI test that makes sure bench jobs can run successfully?

@maflcko
Copy link
Member

maflcko commented Jan 28, 2020

@instagibbs There is, and all benches do run successfully. This fixes an issue where the --filter=.... option no longer works correctly, which we don't check in CI

@instagibbs
Copy link
Member

instagibbs commented Jan 28, 2020

@MarcoFalke ok well that's a suggestion :)

@maflcko
Copy link
Member

maflcko commented Jan 28, 2020

Here is the existing check, which is run in ci:

bitcoin/src/Makefile.test.include

Line 594 in 94c6f2b

$(BENCH_BINARY) -evals=1 -scaling=0 > /dev/null

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 28, 2020

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #18011 (Replace current benchmarking framework with nanobench by martinus)

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.

@cvengler
Copy link
Contributor

cvengler commented Jan 28, 2020

I get this error:

$ src/bench/bench_bitcoin -filter=*
terminate called after throwing an instance of 'std::regex_error'
  what():  regex_error

Am I doing something wrong

@elichai
Copy link
Contributor Author

elichai commented Jan 28, 2020

I get this error:

$ src/bench/bench_bitcoin -filter=*
terminate called after throwing an instance of 'std::regex_error'
  what():  regex_error

Am I doing something wrong

that seems to be unrelated to the bug I'm fixing, the regex engine here(std::regex) isn't great.
Try something like -filter=RIPEMD160|SHA256|SHA1

@elichai elichai mentioned this pull request Jan 28, 2020
Closed
maflcko pushed a commit that referenced this pull request Jan 28, 2020
Merge #18013: bench: Fix benchmarks filters
fe48ac8 
0dae5a5 Fix benchmarks filters (Elichai Turkel)

Pull request description:

  The bug was introduced in #17781
  before this fix `./src/bench/bench_bitcoin -filter=*` will fail with:

  ```
  # Benchmark, evals, iterations, total, min, max, median
  bench_bitcoin: bench/bench.cpp:119: static void benchmark::BenchRunner::RunAll(benchmark::Printer&, uint64_t, double, const string&, bool): Assertion `g_testing_setup == nullptr' failed.
  Aborted (core dumped)
  ```

ACKs for top commit:
  MarcoFalke:
    ACK 0dae5a5

Tree-SHA512: 43de4c7f4a5f29593972cf3bc822429466d0609c159c95d37c9e5370be392ace698b218a65542c7d53bfa52db7377ebdab808501ae109c2249f7f956bd318312
@maflcko maflcko merged commit 0dae5a5 into bitcoin:master Jan 28, 2020
@MarkLTZ MarkLTZ mentioned this pull request Apr 4, 2020
Closed
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 18, 2020
@elichai @PiRK
Fix benchmarks filters
8d80549 
Summary:
> The bug was introduced in [[bitcoin/bitcoin#17781 | PR17781]]
> before this fix `./src/bench/bitcoin-bench -filter="RIPEMD160|SHA256|SHA1"` will fail with:
> ```
> # Benchmark, evals, iterations, total, min, max, median
> bench_bitcoin: bench/bench.cpp:119: static void benchmark::BenchRunner::RunAll(benchmark::Printer&, uint64_t, double, const string&, bool): Assertion `g_testing_setup == nullptr' failed.
> Aborted (core dumped)
> ```

This is a backport of Core [[bitcoin/bitcoin#18013 | PR18013]]

Test Plan: `ninja && ./src/bench/bitcoin-bench -filter="RIPEMD160|SHA256|SHA1"`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D8690
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@elichai @maflcko @instagibbs @DrahtBot @cvengler