Skip to content

Conversation

JeremyRubin
Copy link
Contributor

The prevector tests are really slow when run under wine. This needs to be fixed because it can cause failures during development if one adds new tests timeouts can be hit in travis. This PR adds 3 worker threads + one master to mitigate performance, and removes unnecessary full testing setup. This PR makes the test time go from 205 seconds to 3.7 seconds.

I selected the magic number 4 because that is what I had read in the travis docs as a good number. I also switch from TestingSetup to BasicTestingSetup because TestingSetup spawns unused threads

Prevector tests should be sped up by some other means too, but that doesn't preclude parallelization as done in this PR.

Benchmarks

On a 64-bit windows build off of master, I see:
real 3m24.907s
user 3m24.705s
sys 0m0.038s

After parallelization:
real 0m4.170s
user 0m4.492s
sys 0m9.319s

After parallelization+switch to BasicTestingSetup:
real 0m3.782s
user 0m3.972s
sys 0m6.615s

Analysis

I'm pretty surprised at the speedup; I did test before PR'ing that:

  1. The for loop iterates the correct number of times
  2. Replacing my macros with function-versions had same speedup
  3. Adding a pBOOST_CHECK(false); in the for loop causes a failure

So I feel pretty confident in the correctness that all checks are run and errors reported, but perhaps there is something I've missed. My theory is the speedup has to do with the wine runtime having high overhead with the main thread.

Testing with a single worker thread doing all the checks gives:

real 0m4.682s
user 0m4.444s
sys 0m0.037s

Which seems to confirm my theory (I've left the 4 threaded version because it's marginally faster).

@fanquake fanquake added the Tests label Aug 31, 2016
@sipa
Copy link
Member

sipa commented Aug 31, 2016

What is the explanation for why these tests are so slow under Wine, and why do these changes speed it up so tremendously?

@laanwj
Copy link
Member

laanwj commented Sep 1, 2016

Indeed, the speed-up is much, much more than you'd expect from going to four threads.

What is the most impactful change here? Going from a full testing setup to basic testing setup?

@JeremyRubin
Copy link
Contributor Author

Looking at it with fresh eyes I realized what was making it so slow:

BOOST_CHECK does some internal logging stuff which is costly, which my code guts out (but still logs something). So single threaded should be plenty fast now.

real 0m1.237s
user 0m1.013s
sys 0m0.041s

I didn't notice it initially because I had to replace BOOST_CHECK before parallelizing as it isn't thread safe.

Sorry for the premature parallelization!

Should I push a squashed version?

(Also it may be worth doing this on the whole test suite; which I can do as another patch)

@JeremyRubin
Copy link
Contributor Author

Ignore that last patch for a second.

@JeremyRubin
Copy link
Contributor Author

Should be fine now; was just concerned that I had accidentally shadowed i and that would have an impact; it doesn't.

@JeremyRubin
Copy link
Contributor Author

Found someone else documenting also having BOOST_CHECK being slow http://lists.boost.org/Archives/boost/2008/07/139575.php

@sipa
Copy link
Member

sipa commented Sep 1, 2016 via email

@JeremyRubin JeremyRubin force-pushed the faster_prevector_tests branch from 1faa8c7 to 3ab44ca Compare September 1, 2016 18:15
@JeremyRubin JeremyRubin force-pushed the faster_prevector_tests branch from 3ab44ca to dbb1331 Compare September 1, 2016 18:17
@JeremyRubin
Copy link
Contributor Author

squashed here.

@JeremyRubin
Copy link
Contributor Author

See #8650 for a more general version of this.

@JeremyRubin JeremyRubin closed this Sep 6, 2016
@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.

4 participants