-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Speed up prevector tests by parallelization #8632
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.
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
Conversation
|
What is the explanation for why these tests are so slow under Wine, and why do these changes speed it up so tremendously? |
|
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? |
|
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 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) |
|
Ignore that last patch for a second. |
|
Should be fine now; was just concerned that I had accidentally shadowed i and that would have an impact; it doesn't. |
|
Found someone else documenting also having BOOST_CHECK being slow http://lists.boost.org/Archives/boost/2008/07/139575.php |
|
Please squash :)
|
1faa8c7 to
3ab44ca
Compare
3ab44ca to
dbb1331
Compare
|
squashed here. |
|
See #8650 for a more general version of this. |
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:
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).