Skip to content

Conversation

JeremyRubin
Copy link
Contributor

Tests are really slow because of BOOST_CHECK doing a lot of unnecessary things in wine, BOOST_CHECK can't just be ripped out because some people rely on certain features for there dev process.

In lieu of a more permanent fix, this PR minimally modifies PrevectorTests to use a local check. To make sure this local check isn't useless, it gets the seeds from the insecure random to allow tests to be repeated.

This should be an easy to merge PR; discussion about generally improving the unit test performance/framework beyond this can go here: #8670

@JeremyRubin
Copy link
Contributor Author

Benchmarks

On my local machine on a 32-bit win build running just PrevectorTests goes from taking 4 minutes + 24 seconds to 1.2 seconds with this patch.

@sipa
Copy link
Member

sipa commented Sep 6, 2016

Concept ACK

@TheBlueMatt
Copy link
Contributor

utACK

@fanquake fanquake added the Tests label Sep 7, 2016
@dcousens
Copy link
Contributor

dcousens commented Sep 7, 2016

concept ACk

@JeremyRubin
Copy link
Contributor Author

Travis Benchmarks

Comparing this build to the head build time for make check to run:

Job 2 (i686-w64-mingw32)

678s→337s

data:

https://travis-ci.org/bitcoin/bitcoin/jobs/157997974
https://travis-ci.org/bitcoin/bitcoin/jobs/157884494

Job 4 (x86_64-w64-mingw32)

529s→196s

data:

https://travis-ci.org/bitcoin/bitcoin/jobs/157997976
https://travis-ci.org/bitcoin/bitcoin/jobs/157884498

@jonasschnelli
Copy link
Contributor

utACK f71d4a3

@maflcko
Copy link
Member

maflcko commented Sep 7, 2016

Thanks! utACK f71d4a3

@maflcko maflcko merged commit f71d4a3 into bitcoin:master Sep 8, 2016
maflcko pushed a commit that referenced this pull request Sep 8, 2016
f71d4a3 Minimal fix to slow prevector tests as stopgap measure (Jeremy Rubin)
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Jan 25, 2021
b886a4c Fix header guards using reserved identifiers (Dan Raviv)
b389b3f speed up Unserialize_impl for prevector (Akio Nakamura)
13d2102 Minimal fix to slow prevector tests as stopgap measure (Jeremy Rubin)

Pull request description:

  Backports bitcoin#12324.
  The `DeserializeAndCheckBlock` benchmark (introduced in #2146) shows a speedup of about 4% (not as much as the upstream PR, due to the optimizations already included in #2083).

  Cherry-picks also
  - bitcoin#8671 (with minimal changes to the random context, due to bitcoin#8914 and bitcoin#9792 being already ported out of order).
  - bitcoin#11151

ACKs for top commit:
  Fuzzbawls:
    utACK b886a4c
  furszy:
    utACK b886a4c and merging..

Tree-SHA512: f5de40e5acfb0b875d413d8995d71dd90489730fe4853f0be03d76a1c44ec95eaeb28c0c40d8e91906f23529ad26501bda4f9779ce466cd8603ed97f1662ca98
zkbot added a commit to zcash/zcash that referenced this pull request Feb 17, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants