[tests] make pruning test faster #15686

jnewbery · 2019-03-28T16:13:56Z

This commit makes the pruning.py much faster.

Key insights to do this:

pruning.py doesn't care what kind of transactions make up the big
blocks that are pruned in the test. Instead of making blocks with
several large, expensive to construct and validate transactions,
instead make the large blocks contain a single coinbase transaction with
a huge OP_RETURN txout.
avoid stop-starting nodes where possible.

practicalswift · 2019-03-28T16:51:47Z

Very nice! Do you have any numbers to share? :-)

jnewbery · 2019-03-28T16:59:00Z

Very nice! Do you have any numbers to share? :-)

~30 minutes -> ~4 minutes on my machine.

(I should have mentioned that this is #10591 resurrected)

instagibbs · 2019-03-28T17:52:28Z

test/functional/feature_pruning.py

one hand clapping: can a coinbase tx ever be "non-standard"? :)

I think all coinbases are "non-standard", since we never relay them. :)

Heh. Would you prefer "A transaction containing this scriptPubKey would be non-standard in a non-coinbase transaction, but is consensus valid"?

I also notice that the comment incorrectly says 'scriptsig' instead of 'scriptPubKey'. I'll update.

just a joke :)

Needed to fix other comments, so I've fixed this too. Thanks.

instagibbs

tACK 3ce84d7 with non-blocking nits.

2019-03-28T18:08:04.929000Z TestFramework (INFO): Warning! This test requires 4GB of disk space and takes over 30 mins (up to 2 hours) no longer true as well for me, took far less than 5 minutes on my debug build.

instagibbs · 2019-03-28T17:56:04Z

test/functional/feature_pruning.py

block_hex? I also think a simple .hex() is kosher on master now(or at least I see many incantations of it!)

Also just as test belt and suspenders, make sure the blocks you are making are actually the size you think they are:

assert_greater_than(len(block_hex), len(big_script)*2)

or there abouts

Thanks. I've changed this to use ToHex() to match other tests.

jnewbery · 2019-03-28T19:44:04Z

Force pushed some minor fixups. I also tested sending the blocks over P2P, and it's actually slower, so I've removed the comment saying we should do that.

instagibbs · 2019-03-28T19:48:59Z

reACK ba39546

sipa · 2019-03-28T21:27:48Z

Concept ACK

maflcko · 2019-03-28T22:36:24Z

~30 minutes -> ~4 minutes on my machine.

That's impressive. Mind to move it to the non-extended tests?

DrahtBot · 2019-03-28T22:55:40Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#15680 ([WIP] Remove resendwallettransactions RPC method by jnewbery)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

jnewbery · 2019-03-28T23:46:19Z

Mind to move it to the non-extended tests?

Sadly, I think this still won't run on travis. Here's my comment on the original PR:

Unfortunately, pruning.py still isn't able to run on Travis. It appears that disk access is very slow for in the Travis environment, and this test still times out/fails. Unfortunately, it's not possible to run this test without very heavy disk access - the whole point of pruning is to remove block files when disk usage goes over a certain threshold.

You've also asked that I split out the style changes into a separate commit, which is a fair request. I'll do that tomorrow.

Minor style fixups. No functional change.

This commit makes the pruning.py much faster. Key insights to do this: - pruning.py doesn't care what kind of transactions make up the big blocks that are pruned in the test. Instead of making blocks with several large, expensive to construct and validate transactions, instead make the large blocks contain a single coinbase transaction with a huge OP_RETURN txout. - avoid stop-starting nodes where possible. This test could probably be made even faster by using the P2P interface for submitting blocks instead of the submitblock RPC.

jnewbery · 2019-03-29T15:45:44Z

I've split style-only changes into their own commit, to make review easier. Pushed branch should be identical to previous.

maflcko · 2019-03-29T16:20:47Z

utACK 03d6d23

maflcko · 2019-03-29T17:16:32Z

Time: Less than 3 minutes on ssd and about 4 minutes on hdd. 🚀

03d6d23 [tests] make pruning test faster (John Newbery) 1c29ac4 [tests] style fixes in feature_pruning.py (John Newbery) Pull request description: This commit makes the pruning.py much faster. Key insights to do this: - pruning.py doesn't care what kind of transactions make up the big blocks that are pruned in the test. Instead of making blocks with several large, expensive to construct and validate transactions, instead make the large blocks contain a single coinbase transaction with a huge OP_RETURN txout. - avoid stop-starting nodes where possible. ACKs for commit 03d6d2: MarcoFalke: utACK 03d6d23 Tree-SHA512: 511642ce0fa294319dce3486fe06d75970d8ab66deda7f692be081d3056b4ce5b4cf91a7b5762eefbba224ba6c848750016454ff1e5d564acc507b1c41213628

instagibbs · 2019-03-29T17:21:37Z

re-ACK 03d6d23

ryanofsky

ACK 03d6d23. Test also took ~3 minutes for me (on ssd)

ryanofsky · 2019-03-29T17:10:39Z

test/functional/feature_pruning.py


 WARNING:
 This test uses 4GB of disk space.
 This test takes 30 mins or more (up to 2 hours)


Could update this comment

ryanofsky · 2019-03-29T17:23:11Z

test/functional/feature_pruning.py

+        block.nVersion = best_block["version"]
+        block.hashPrevBlock = previousblockhash
+        block.nTime = mine_large_blocks.nTime
+        block.nBits = int('207fffff', 16)


Could write 0x207fffff

jnewbery · 2019-03-29T17:39:22Z

Thanks for the review @ryanofsky . This has been merged so I'm not intending to make further changes (happy to review your PR if you want to make the changes though).

03d6d23 [tests] make pruning test faster (John Newbery) 1c29ac4 [tests] style fixes in feature_pruning.py (John Newbery) Pull request description: This commit makes the pruning.py much faster. Key insights to do this: - pruning.py doesn't care what kind of transactions make up the big blocks that are pruned in the test. Instead of making blocks with several large, expensive to construct and validate transactions, instead make the large blocks contain a single coinbase transaction with a huge OP_RETURN txout. - avoid stop-starting nodes where possible. ACKs for commit 03d6d2: MarcoFalke: utACK 03d6d23 Tree-SHA512: 511642ce0fa294319dce3486fe06d75970d8ab66deda7f692be081d3056b4ce5b4cf91a7b5762eefbba224ba6c848750016454ff1e5d564acc507b1c41213628

merge bitcoin#15685,bitcoin#15335,bitcoin#15642,bitcoin#15686,bitcoin#15629

03d6d23 [tests] make pruning test faster (John Newbery) 1c29ac4 [tests] style fixes in feature_pruning.py (John Newbery) Pull request description: This commit makes the pruning.py much faster. Key insights to do this: - pruning.py doesn't care what kind of transactions make up the big blocks that are pruned in the test. Instead of making blocks with several large, expensive to construct and validate transactions, instead make the large blocks contain a single coinbase transaction with a huge OP_RETURN txout. - avoid stop-starting nodes where possible. ACKs for commit 03d6d2: MarcoFalke: utACK 03d6d23 Tree-SHA512: 511642ce0fa294319dce3486fe06d75970d8ab66deda7f692be081d3056b4ce5b4cf91a7b5762eefbba224ba6c848750016454ff1e5d564acc507b1c41213628

jnewbery mentioned this pull request Mar 28, 2019

Remove resendwallettransactions RPC method #15680

Merged

instagibbs reviewed Mar 28, 2019

View reviewed changes

instagibbs approved these changes Mar 28, 2019

View reviewed changes

jnewbery force-pushed the 2019_03_faster_pruning_test branch from 3ce84d7 to ba39546 Compare March 28, 2019 19:41

DrahtBot added the Tests label Mar 28, 2019

jnewbery added 2 commits March 29, 2019 11:43

[tests] style fixes in feature_pruning.py

1c29ac4

Minor style fixups. No functional change.

jnewbery force-pushed the 2019_03_faster_pruning_test branch from ba39546 to 03d6d23 Compare March 29, 2019 15:44

maflcko merged commit 03d6d23 into bitcoin:master Mar 29, 2019

ryanofsky reviewed Mar 29, 2019

View reviewed changes

jnewbery mentioned this pull request Jul 8, 2019

tests: Speedup feature_pruning test and refactor big transaction logic #14691

Closed

adamjonas mentioned this pull request Oct 18, 2019

[tests] make pruning.py faster #10591

Closed

jnewbery deleted the 2019_03_faster_pruning_test branch October 18, 2019 20:28

PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 21, 2021

Merge pull request #4484 from vijaydasmp/backport_v19_vijay_batch_1

6e32a46

merge bitcoin#15685,bitcoin#15335,bitcoin#15642,bitcoin#15686,bitcoin#15629

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

[tests] make pruning test faster #15686

[tests] make pruning test faster #15686

Uh oh!

Conversation

jnewbery commented Mar 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Mar 28, 2019

Uh oh!

jnewbery commented Mar 28, 2019

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!

instagibbs left a comment

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!

jnewbery commented Mar 28, 2019

Uh oh!

instagibbs commented Mar 28, 2019

Uh oh!

sipa commented Mar 28, 2019

Uh oh!

maflcko commented Mar 28, 2019

Uh oh!

DrahtBot commented Mar 28, 2019

Conflicts

Uh oh!

jnewbery commented Mar 28, 2019

Uh oh!

jnewbery commented Mar 29, 2019

Uh oh!

maflcko commented Mar 29, 2019

Uh oh!

maflcko commented Mar 29, 2019

Uh oh!

instagibbs commented Mar 29, 2019

Uh oh!

ryanofsky left a comment

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!

jnewbery commented Mar 29, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jnewbery commented Mar 28, 2019 •

edited

Loading