travis: Run bench_bitcoin once #13811

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1808-travisBench changing 1 files +3 −0

MarcoFalke commented at 7:18 pm on July 30, 2018: member

The bench should be run once just to make sure it doesn’t crash or hit assertions.

Closes: #13810
MarcoFalke force-pushed on Jul 30, 2018
MarcoFalke force-pushed on Jul 30, 2018
fanquake added the label Tests on Jul 30, 2018
DrahtBot commented at 3:14 pm on July 31, 2018: member
- #13816 (travis: build and run tests on os: osx by scravy)
- #13515 (travis: Enable Qt build for Windows , i686 and arm Linux by ken2812221)
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.
scravy commented at 1:59 pm on August 1, 2018: contributor

WARNING: This is a debug build - may result in slower benchmarks.

Maybe it would make more sense to introduce an additional stage “benchmark” which does a non-debug build and perform the benchmark in there?

Also it would clearly separate it visually when looking at travis. The only thing that sucks about this approach that it requires an additional build = additional travis run time. But given the extensive caching being done this should be okay.
MarcoFalke commented at 2:07 pm on August 1, 2018: member

Maybe it would make more sense to introduce an additional stage “benchmark” which does a non-debug build and perform the benchmark in there?

Please see https://bitcoinperf.com/ for these. The one here is specifically designed to run everything in less than a 1/10 of a second. (scaling=0.001)
scravy commented at 2:14 pm on August 1, 2018: contributor

Ah, cool! Didn’t know about this site.

Since this site does exist – what is the purpose of running the benchmark in the CI pipeline though? Is it to check that a commit didn’t break the benchmark, i.e. to check that is runs successfully, not how fast actually?

I think as a contributor who’s new to the project I would love to see a comment communicating that intent next to it (if this is the intent). Some of the lines in the .travis.yml are incomprehensible (for example I have no idea which steps require the before-install step of removing everything that contains /opt/python from the path). That should help maintain it.
travis: Run bench_bitcoin once fa7a3a1783
MarcoFalke force-pushed on Aug 1, 2018
MarcoFalke commented at 2:26 pm on August 1, 2018: member

Updated commit and OP based on your feedback
jamesob commented at 8:47 pm on August 1, 2018: member

utACK https://github.com/bitcoin/bitcoin/pull/13811/commits/fa7a3a1783cd81907779392f626bdcca5e10efb1
scravy commented at 7:46 am on August 2, 2018: contributor

ACK fa7a3a1783cd81907779392f626bdcca5e10efb1
ken2812221 commented at 8:59 am on August 2, 2018: contributor

utACK fa7a3a1
laanwj commented at 10:34 am on August 2, 2018: member

utACK fa7a3a1783cd81907779392f626bdcca5e10efb1

The one here is specifically designed to run everything in less than a 1/10 of a second. (scaling=0.001)

Very good idea.
laanwj merged this on Aug 2, 2018
laanwj closed this on Aug 2, 2018
laanwj referenced this in commit 660abc1713 on Aug 2, 2018
MarcoFalke deleted the branch on Aug 2, 2018
MarcoFalke locked this on Sep 8, 2021

Contributors
MarcoFalke DrahtBot scravy jamesob ken2812221 laanwj

Labels
Tests