Skip to content
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

[Core] Prevector #1557

Merged
merged 12 commits into from
May 19, 2020
Merged

[Core] Prevector #1557

merged 12 commits into from
May 19, 2020

Conversation

random-zebra
Copy link

@random-zebra random-zebra commented Apr 24, 2020

Based on:

This introduces prevector<N, T>, a new basic data type which is a fully API compatible drop-in replacement for std::vector<T> and uses it for the script, to reduce the considerable memory overhead of vectors in cases where they normally contain a small number of small elements.

Original tests in Bitcoin showed a reduction in dbcache memory usage by 23%, and made an initial sync 13% faster.

Backported from the following upstream's PRs, with only additional edits in the 2nd layer scripts (masternode-payments, masternode-budget) and in the unit tests (to address the differences in insecure_rand):

NOTE: Updates to memusage.h needed as well, when #1531 is merged.

Fixes a bug in which pop_back did not call the deleted item's
destructor.

Using the most general erase() implementation to implement all the
others
prevents similar bugs because the coupling between deallocation and
destructor
invocation only needs to be maintained in one place.
Also reduces duplication of complex memmove logic.

backports bitcoin/bitcoin@1e2c29f
swap was using an incorrect condition to determine when to apply an
optimization
(not swapping the full direct[] when swapping two indirect prevectors).

Rather than correct the optimization I'm removing it for simplicity.
Removing
this optimization minutely improves performance in the typical
(currently only)
usage of member swap(), which is swapping with a freshly
value-initialized
object.

backports bitcoin/bitcoin@a7af72a
- add a swap operation to prevector tests (fails due to broken
prevector::swap)
- fix 2 prevector test operation conditions that were impossible

backports bitcoin/bitcoin@4ed41a2
This returns a pointer to the beginning of the vector's data.

backports bitcoin/bitcoin@47314e6
Implement `begin_ptr` and `end_ptr` in terms of C++11 code,
and add a comment that they are deprecated.

backports bitcoin/bitcoin@f00705a
Such moves are used when reallocating vectors that contain them,
for example.

backports bitcoin/bitcoin@2ddfcfd
@random-zebra
Copy link
Author

Rebased and added prevector's DynamicUsage, now that both #1531 and #1554 have been merged. Ready for review/QA

@furszy
Copy link

furszy commented May 18, 2020

Concept and code ACK, awesome integration 🚀 .
Unit and functional test are all passing here.

Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK a8b965e .

@ambassador000
Copy link

Functionality tested, working as intended.

@Fuzzbawls Fuzzbawls added this to Ready in perpetual updating PIVX Core to BTC Core via automation May 19, 2020
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a8b965e

furszy added a commit that referenced this pull request Jun 8, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants