Skip to content

[Trivial] Add a comment on the use of prevector in script. #11011

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 15, 2017
Merged

[Trivial] Add a comment on the use of prevector in script. #11011

merged 1 commit into from
Aug 15, 2017

Conversation

gmaxwell
Copy link
Contributor

@gmaxwell gmaxwell commented Aug 8, 2017

No description provided.

@fanquake fanquake added the Docs label Aug 8, 2017
paveljanik
paveljanik reviewed Aug 9, 2017
View reviewed changes
src/script/script.h Outdated
* We use a prevector for the script to reduce the considerable memory overhead
* of vectors in cases where they normally contain a small number of small elements.
* Tests in October 2015 showed use of this reduced dbcache memory usage by 23%
* and and made an initial sync 13% faster.
Copy link
Contributor

Choose a reason for hiding this comment

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

and and

@instagibbs
Copy link
Member

ACK(and and aside), very helpful!

@sdaftuar
Copy link
Member

ACK (apart from @paveljanik's nit)

@fanquake
Copy link
Member

utACK post nit

promag
promag reviewed Aug 15, 2017
View reviewed changes
src/script/script.h
@@ -377,6 +377,12 @@ class CScriptNum
int64_t m_value;
};

/**
* We use a prevector for the script to reduce the considerable memory overhead
Copy link
Contributor

Choose a reason for hiding this comment

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

nanonit: * Use a prevector ...

@maflcko maflcko merged commit 08f71c2 into bitcoin:master Aug 15, 2017
maflcko pushed a commit that referenced this pull request Aug 15, 2017
Merge #11011: [Trivial] Add a comment on the use of prevector in script.
7db65c3 
08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
@sickpig sickpig mentioned this pull request Oct 1, 2017
Merged
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 19, 2019
@PastaPastaPasta
Merge bitcoin#11011: [Trivial] Add a comment on the use of prevector …
ae25cb7 
…in script.

08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 23, 2019
@PastaPastaPasta
Merge bitcoin#11011: [Trivial] Add a comment on the use of prevector …
89dd77f 
…in script.

08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 24, 2019
@PastaPastaPasta
Merge bitcoin#11011: [Trivial] Add a comment on the use of prevector …
d57fd7c 
…in script.

08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Nov 19, 2019
@PastaPastaPasta
Merge bitcoin#11011: [Trivial] Add a comment on the use of prevector …
88f5060 
…in script.

08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Nov 21, 2019
@PastaPastaPasta
Merge bitcoin#11011: [Trivial] Add a comment on the use of prevector …
a4a6e92 
…in script.

08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 9, 2019
@PastaPastaPasta
Merge bitcoin#11011: [Trivial] Add a comment on the use of prevector …
b7af9ee 
…in script.

08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 1, 2020
@PastaPastaPasta
Merge bitcoin#11011: [Trivial] Add a comment on the use of prevector …
2e8e319 
…in script.

08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 2, 2020
@PastaPastaPasta
Merge bitcoin#11011: [Trivial] Add a comment on the use of prevector …
972a049 
…in script.

08f71c2 [Trivial] Add a comment on the use of prevector in script. (Gregory Maxwell)

Pull request description:

Tree-SHA512: 020981516e67e576685eb9a8532178fb97d1780af409fc86d869cd05c293c0c823c26e838cf544d18610f5a3f479ce3e47d2ccb95fb1c4e55fe9e7ceb354f20b
@random-zebra random-zebra mentioned this pull request Apr 24, 2020
Merged
1 task
Fuzzbawls added a commit to PIVX-Project/PIVX that referenced this pull request May 19, 2020
@Fuzzbawls
Merge #1557: [Core] Prevector
8dfc480 
a8b965e Explicitly initialize prevector _union (random-zebra)
bdd98e8 [Trivial] Add a comment on the use of prevector in script. (random-zebra)
bbaa6e3 Clarify prevector::erase and avoid swap-to-clear (random-zebra)
28a8435 prevector: assert successful allocation (random-zebra)
de41cb5 Only call clear on prevector if it isn't trivially destructible (random-zebra)
8784df3 Make CScript (and prevector) c++11 movable. (random-zebra)
83f9ac6 serialize: Deprecate `begin_ptr` / `end_ptr` (random-zebra)
c3ecc12 prevector: add C++11-like data() method (random-zebra)
5490aa0 test prevector::swap (random-zebra)
035760e prevector::swap: fix (unreached) data corruption (random-zebra)
0e71400 prevector: destroy elements only via erase() (random-zebra)
9811a68 [Core] Prevector type (random-zebra)

Pull request description:

  Based on:
  - [x] #1554

  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`):

  - bitcoin#6914
  - bitcoin#7888
  - bitcoin#8850
  - bitcoin#9349
  - bitcoin#9505
  - bitcoin#9856
  - bitcoin#10534
  - bitcoin#11011
  - bitcoin#14028

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

ACKs for top commit:
  furszy:
    Tested ACK a8b965e .
  Fuzzbawls:
    ACK a8b965e

Tree-SHA512: 203b660dd8d9f98780be660dae0ac951766ba438836bd6f0ec921e7749c1a84143f3b920fe49f2cc399f4aa7e763098f78746d3b6ff845bcf913bb13de984bc1
@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.
Reviewers
3 more reviewers

@promag promag promag left review comments

@paveljanik paveljanik paveljanik left review comments

@kallewoof kallewoof kallewoof approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@gmaxwell @instagibbs @sdaftuar @fanquake @kallewoof @promag @paveljanik @maflcko