compressor: use a prevector in CompressScript serialization [ZAP1]#18847
Merged
maflcko merged 1 commit intobitcoin:masterfrom Apr 28, 2021
Merged
compressor: use a prevector in CompressScript serialization [ZAP1]#18847maflcko merged 1 commit intobitcoin:masterfrom
maflcko merged 1 commit intobitcoin:masterfrom
Conversation
Contributor
|
There's also a call in bitcoin/src/test/fuzz/script.cpp Lines 39 to 40 in 844d207 |
Empact
reviewed
May 2, 2020
Member
|
Any reason not to do this for decompression? |
Contributor
Author
nope, I could do that as well. |
jb55
force-pushed
the
2020-05-compresscript-prevector
branch
4 times, most recently
from
3ac60ed to
14cd3d9
Compare
Empact
reviewed
May 15, 2020
jb55
force-pushed
the
2020-05-compresscript-prevector
branch
3 times, most recently
from
0616c3d to
d03a694
Compare
JeremyRubin
reviewed
May 15, 2020
jb55
force-pushed
the
2020-05-compresscript-prevector
branch
2 times, most recently
from
b782989 to
98c0688
Compare
JeremyRubin
reviewed
May 15, 2020
Contributor
Author
|
Can you explain where 33 comes from?
oh yeah good idea
|
jb55
force-pushed
the
2020-05-compresscript-prevector
branch
2 times, most recently
from
30834be to
6183bda
Compare
Use a prevector for stack allocation instead of heap allocation during script compression and decompression. These functions were doing millions of unnecessary heap allocations during IBD. We introduce a CompressedScript type alias for this prevector. It is size 33 as that is the maximum size of a compressed script. Fix the DecompressScript header to match the variable name from compressor.cpp Signed-off-by: William Casarin <[email protected]>
jb55
force-pushed
the
2020-05-compresscript-prevector
branch
from
6183bda to
83a425d
Compare
jb55
changed the title
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
jb55
changed the title
Contributor
|
ACK 83a425d |
Contributor
|
tACK 83a425d |
Contributor
Author
|
doing some tests I'm seeing a very tiny improvement in tmpfs/memdisk reindex up to height 200k: but could just be luck 🤷 would be interesting to test this to even higher heights. I also traced the number of calls to CompressScript: around I used
|
Contributor
Author
|
friendly yearly bump |
Member
|
utACK 83a425d |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Apr 29, 2021
gwillen
pushed a commit
to ElementsProject/elements
that referenced
this pull request
Jun 1, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This function was doing millions of unnecessary heap allocations during IBD.
I'm start to catalog unnecessary heap allocations as a pet project of mine: as-zero-as-possible-alloc IBD. This is one small step.
before:
after:
should I type alias this?I type aliased itThis is a part of the Zero Allocations Project #18849 (ZAP1). This code came up as a place where many allocations occur.