Basic CCheckQueue Benchmarks #9498

JeremyRubin · 2017-01-09T19:06:28Z

Some basic benchmarks for the CCheckQueue, for future work to compare to.

ryanofsky

utACK 9f03110. Looks fine, just minor comments.

ryanofsky · 2017-01-11T21:00:19Z

src/bench/checkqueue.cpp

+// weight Checks, so it should make any lock contention
+// particularly visible
+static void CCheckQueueSpeed(benchmark::State& state)
+{


Maybe declare some constants here with the various magic numbers & things that could be tuned in the benchmark, like size of the queue, number of threads, number of jobs to add, number of times to call add, etc.

ryanofsky · 2017-01-11T21:05:21Z

src/bench/checkqueue.cpp

+    while (state.KeepRunning()) {
+        CCheckQueueControl<FakeJobNoWork> control(&queue);
+        // We can make vChecks out of the loop because calling Add doesn't
+        // change the size of the vector.


Maybe add an assert to the end of the benchmark to check this? It's be easy to imagine the vector being moved from in the future, which would invalidate the benchmark.

ryanofsky · 2017-01-11T21:11:11Z

src/bench/checkqueue.cpp

+    while (state.KeepRunning()) {
+        CCheckQueueControl<FakeJobNoWork> control(&queue);
+        // We can make vChecks out of the loop because calling Add doesn't
+        // change the size of the vector.


Also, if this is true, any reason not to move vChecks declaration out of the while loop too?

ryanofsky · 2017-01-11T21:13:12Z

src/bench/checkqueue.cpp

+        // We can make vChecks out of the loop because calling Add doesn't
+        // change the size of the vector.
+        std::vector<FakeJobNoWork> vChecks;
+        vChecks.resize(30);


Could call the vector constructor taking a size to eliminate the resize line.

ryanofsky · 2017-01-11T21:18:15Z

src/bench/checkqueue.cpp

+    }
+    while (state.KeepRunning()) {
+        // Make insecure_rand here so that each iteration is identical.
+        FastRandomContext insecure_rand(true);


Maybe move this outside while loop, if whatever time this takes isn't meaningful to the benchmark.

It is meaningful for each iteration to be identical.

Oops, should have read the comment above. Though maybe I would reword it to something like "Start with a fresh insecure_rand here" rather than "Make insecure_rand here."

ryanofsky · 2017-01-11T21:24:12Z

src/bench/checkqueue.cpp

+        PrevectorJob(){
+        }
+        PrevectorJob(FastRandomContext& insecure_rand){
+            p.resize(insecure_rand.rand32() % 56);


Why do this in the constructor instead of the call operator? Wouldn't it be more realistic to have the work done in the worker threads instead of the control thread? Maybe add a comment if this is the intended behavior.

FastRandomContext is not thread safe.

Also we do want the main thread to be doing some work while the worker threads process.

…c numbers, fixed scoping of vectors (and memory movement component of benchmark).

ryanofsky

utACK aad4cb5

fanquake · 2017-01-12T01:59:39Z

CCheckQueueSpeed,576,0.001812756061554,0.001984249800444,0.001944298545520,6149498,6731089,6595898
CCheckQueueSpeed,576,0.001704573631287,0.001963026821613,0.001897984494766,5782270,6659114,6438475
CCheckQueueSpeed,576,0.001838877797127,0.001967173069715,0.001926153070397,6237919,6675994,6534347
CCheckQueueSpeed,576,0.001697123050690,0.001930065453053,0.001841966890626,5757193,6547343,6248762
CCheckQueueSpeed,576,0.001837179064751,0.001974612474442,0.001906548523241,6232267,6698444,6467841

CCheckQueueSpeedPrevectorJob,448,0.002283558249474,0.002333313226700,0.002298991062811,7746465,7915155,7799211
CCheckQueueSpeedPrevectorJob,448,0.002310715615749,0.002362936735153,0.002330580460174,7838558,8015741,7906361
CCheckQueueSpeedPrevectorJob,448,0.002285875380039,0.002489343285561,0.002338334918022,7754281,8444561,7932261
CCheckQueueSpeedPrevectorJob,448,0.002281628549099,0.002457186579704,0.002311725169420,7739965,8335454,7842407
CCheckQueueSpeedPrevectorJob,448,0.002286218106747,0.002332687377930,0.002305187284946,7755492,7913158,7820222

laanwj · 2017-01-19T14:29:07Z

utack aad4cb5

aad4cb5 Address ryanofsky feedback on CCheckQueue benchmarks. Eliminated magic numbers, fixed scoping of vectors (and memory movement component of benchmark). (Jeremy Rubin) 9f03110 Add Basic CheckQueue Benchmark (Jeremy Rubin)

Micro-benchmarking framework part 1 Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6733 - bitcoin/bitcoin#6770 - bitcoin/bitcoin#6892 - Excluding changes to `src/policy/policy.h` which we don't have yet. - bitcoin/bitcoin#7934 - Just the benchmark, not the performance improvements. - bitcoin/bitcoin#8039 - bitcoin/bitcoin#8107 - bitcoin/bitcoin#8115 - bitcoin/bitcoin#8914 - Required resolving several merge conflicts in code that had been refactored upstream. The changes were simple enough that I decided it was okay to impose merge conflicts on pulling in those refactors later. - bitcoin/bitcoin#9200 - bitcoin/bitcoin#9202 - Adds support for measuring CPU cycles, which is later removed in an upstream PR after the refactor. I am including it to reduce future merge conflicts. - bitcoin/bitcoin#9281 - Only changes to `src/bench/bench.cpp` - bitcoin/bitcoin#9498 - bitcoin/bitcoin#9712 - bitcoin/bitcoin#9547 - bitcoin/bitcoin#9505 - Just the benchmark, not the performance improvements. - bitcoin/bitcoin#9792 - Just the benchmark, not the performance improvements. - bitcoin/bitcoin#10272 - bitcoin/bitcoin#10395 - Only changes to `src/bench/` - bitcoin/bitcoin#10735 - Only changes to `src/bench/base58.cpp` - bitcoin/bitcoin#10963 - bitcoin/bitcoin#11303 - Only the benchmark backend change. - bitcoin/bitcoin#11562 - bitcoin/bitcoin#11646 - bitcoin/bitcoin#11654 This pulls in all changes to the micro-benchmark framework prior to December 2017, when it was rewritten. The rewrite depends on other upstream PRs we have not pulled in yet. This does not pull in all benchmarks prior to December 2017. It leaves out benchmarks that either test code we do not have yet (except for the `FastRandomContext` refactor, which I decided to pull in), or would require rewrites to work with our changes to the codebase.

3f3edde [Bench] Use PIVX address in Base58Decode test (random-zebra) 5a1be90 [Travis] Disable benchmark framework for trusty test (random-zebra) 1bd89ac Initialize recently introduced non-static class member lastCycles to zero in constructor (random-zebra) ec60671 Require a steady clock for bench with at least micro precision (random-zebra) 84069ce bench: prefer a steady clock if the resolution is no worse (random-zebra) 38367b1 bench: switch to std::chrono for time measurements (random-zebra) a24633a Remove countMaskInv caching in bench framework (random-zebra) 9e9bc22 Restore default format state of cout after printing with std::fixed/setprecision (random-zebra) 3dd559d Avoid static analyzer warnings regarding uninitialized arguments (random-zebra) e85f224 Replace boost::function with std::function (C++11) (random-zebra) 98c0857 Prevent warning: variable 'x' is uninitialized (random-zebra) 7f0d4b3 FastRandom benchmark (random-zebra) d9fa0c6 Add prevector destructor benchmark (random-zebra) e1527ba Assert that what might look like a possible division by zero is actually unreachable (random-zebra) e94cf15 bench: Fix initialization order in registration (random-zebra) 151c25f Basic CCheckQueue Benchmarks (random-zebra) 51aedbc Use std:thread:hardware_concurrency, instead of Boost, to determine available cores (random-zebra) d447613 Use real number of cores for default -par, ignore virtual cores (random-zebra) 9162a56 [Refactoring] Removed using namespace <xxx> from bench/ sources (random-zebra) 5c07f67 bench: Add support for measuring CPU cycles (random-zebra) 41ce1ed bench: Fix subtle counting issue when rescaling iteration count (random-zebra) 68ea794 Avoid integer division in the benchmark inner-most loop. (random-zebra) 3fa4f27 bench: Added base58 encoding/decoding benchmarks (random-zebra) 4442118 bench: Add crypto hash benchmarks (random-zebra) a5179b6 [Trivial] ensure minimal header conventions (random-zebra) 8607d6b Support very-fast-running benchmarks (random-zebra) 4aebb60 Simple benchmarking framework (random-zebra) Pull request description: Introduces the benchmarking framework, loosely based on google's micro-benchmarking library (https://github.com/google/benchmark), ported from Bitcoin, up to 0.16. The benchmark framework is hard-coded to run each benchmark for one wall-clock second, and then spits out .csv-format timing information to stdout. Backported PR: - bitcoin#6733 - bitcoin#6770 - bitcoin#6892 - bitcoin#8039 - bitcoin#8107 - bitcoin#8115 - bitcoin#9200 - bitcoin#9202 - bitcoin#9281 - bitcoin#6361 - bitcoin#10271 - bitcoin#9498 - bitcoin#9712 - bitcoin#9547 - bitcoin#9505 (benchmark only. Rest was in #1557) - bitcoin#9792 (benchmark only. Rest was in #643) - bitcoin#10272 - bitcoin#10395 (base58 only) - bitcoin#10963 - bitcoin#11303 (first commit) - bitcoin#11562 - bitcoin#11646 - bitcoin#11654 Current output of `src/bench/bench_pivx`: ``` #Benchmark,count,min(ns),max(ns),average(ns),min_cycles,max_cycles,average_cycles Base58CheckEncode,131072,7697,8065,7785,20015,20971,20242 Base58Decode,294912,3305,3537,3454,8595,9198,8981 Base58Encode,180224,5498,6020,5767,14297,15652,14994 CCheckQueueSpeed,320,3159960,3535173,3352787,8216030,9191602,8717388 CCheckQueueSpeedPrevectorJob,96,9184484,11410840,10823070,23880046,29668680,28140445 FastRandom_1bit,320,3143690,4838162,3199156,8173726,12579373,8317941 FastRandom_32bit,60,17097612,17923669,17367440,44454504,46602306,45156079 PrevectorClear,3072,334741,366618,346731,870340,953224,901516 PrevectorDestructor,2816,344233,368912,357281,895022,959187,928948 RIPEMD160,288,3404503,3693917,3577774,8851850,9604334,9302363 SHA1,384,2718128,2891558,2802513,7067238,7518184,7286652 SHA256,176,6133760,6580005,6239866,15948035,17108376,16223916 SHA512,240,4251468,4358706,4313463,11054006,11332826,11215186 Sleep100ms,10,100221470,100302411,100239073,260580075,260790726,260625870 ``` NOTE: Not all the tests have been pulled yet (as we might not have the code being tested, or it would require rewrites to work with our different code base), but the framework is updated to December 2017. ACKs for top commit: Fuzzbawls: ACK 3f3edde Tree-SHA512: c283311a9accf6d2feeb93b185afa08589ebef3f18b6e86980dbc3647b9845f75ac9ecce2f1b08738d25ceac36596a2c89d41e4dbf3b463502aa695611aa1f8e

Add Basic CheckQueue Benchmark

9f03110

JeremyRubin force-pushed the checkqueue_bench branch from 6762142 to 9f03110 Compare January 9, 2017 19:07

fanquake added the Tests label Jan 9, 2017

ryanofsky approved these changes Jan 11, 2017

View reviewed changes

Address ryanofsky feedback on CCheckQueue benchmarks. Eliminated magi…

aad4cb5

…c numbers, fixed scoping of vectors (and memory movement component of benchmark).

ryanofsky approved these changes Jan 11, 2017

View reviewed changes

laanwj merged commit aad4cb5 into bitcoin:master Jan 19, 2017

JeremyRubin mentioned this pull request Mar 7, 2017

Lock-Free CheckQueue #9938

Closed

str4d mentioned this pull request Feb 22, 2019

Micro-benchmarking framework part 1 zcash/zcash#3858

Merged

sickpig mentioned this pull request Apr 28, 2020

Update benchmark suite BitcoinUnlimited/BitcoinUnlimited#2191

Merged

random-zebra mentioned this pull request May 27, 2020

[Tests] Benchmarking Framework PIVX-Project/PIVX#1650

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Basic CCheckQueue Benchmarks #9498

Basic CCheckQueue Benchmarks #9498

Uh oh!

Conversation

JeremyRubin commented Jan 9, 2017

Uh oh!

ryanofsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake commented Jan 12, 2017

Uh oh!

laanwj commented Jan 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ryanofsky left a comment •

edited

Loading