In my benchmarks, using this pool allocator for CCoinsMap gives about
20% faster `-reindex-chainstate` with -dbcache=5000 with practically the
same memory usage. The change in max RSS changed was 0.3%.
The `validation_flush_tests` tests need to be updated because
memory allocation is now done in large pools instead of one node at a
time, so the limits need to be updated accordingly.
4275195606 De-duplicate add_coin methods to a test util helper (Jon Atack)
9d92c3d7f4 Create InsecureRandMoneyAmount() test util helper (Jon Atack)
81f5ade2a3 Move random test util code from setup_common to random (Jon Atack)
Pull request description:
- Move random test utilities from `setup_common` to a new `random` file, as many tests don't use this code.
- Create a helper to generate semi-random CAmounts up to `MONEY_RANGE` rather than only uint32, and use the helper in the unit tests.
- De-duplicate a shared `add_coin` method by extracting it to a `coins` test utility.
ACKs for top commit:
pinheadmz:
ACK 4275195606
achow101:
ACK 4275195606
john-moffett:
ACK 4275195606
Tree-SHA512: 3ed974251149c7417f935ef2f8865aa0dcc33b281b47522b0f96f1979dff94bb8527957f098fe4d210f40d715c00f29512f2ffe189097102229023b7284a3a27
Add CoinsViewOptions struct to remove ArgsManager uses from txdb.
To reduce size of this commit, this moves references to gArgs variable out of
txdb.cpp to calling code in validation.cpp. But these moves are temporary. The
gArgs references in validation.cpp are moved out to calling code in init.cpp in
later commits.
This commit does not change behavior.
1d7935b45a test: add test for coins view flush behavior using Sync() (James O'Beirne)
2c3cbd6c00 test: add use of Sync() to coins tests (James O'Beirne)
6d8affca96 test: refactor: clarify the coins simulation (James O'Beirne)
79cedc36af coins: add Sync() method to allow flush without cacheCoins drop (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal
---
In certain circumstances, we may want to flush chainstate data to disk without
emptying `cacheCoins`, which affects performance. UTXO snapshot
activation is one such case, as we populate `cacheCoins` with the snapshot
contents and want to persist immediately afterwards but also enter IBD.
See also #15265, which makes the case that under normal operation a
flush-without-erase doesn't necessarily add much benefit. I open this PR
even in light of the previous discussion because (i) flush-without-erase
almost certainly provides benefit in the case of snapshot activation (especially
on spinning disk hardware) and (ii) this diff is fairly small and gives us convenient
options for more granular cache management without changing existing policy.
See also #15218.
ACKs for top commit:
sipa:
ACK 1d7935b45a
achow101:
ACK 1d7935b45a
Sjors:
tACK 1d7935b45a
Tree-SHA512: 897583963e98661767d2d09c9a22f6019da24125558cd88770bfe2d017d924f23a9075b729e4b1febdec5b0709a38e8fa1ef94d62aa88650556b06cb4826c845
In certain circumstances, we may want to flush to disk without
emptying `cacheCoins`, which affects performance. UTXO snapshot
activation is one such case.
This method is currently unused and this commit does not
change any behavior.
Incorporates feedback from John Newbery.
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
Since the removal of NODISCARD in 81d5af42f4,
the only attributes def is LIFETIMEBOUND, and it's included in many more
places that it is used.
This removes all includes which do not have an associated use of LIFETIMEBOUND,
and adds it to the following files, due to their use of the same:
* src/validationinterface.h
* src/script/standard.h
Also re-`static`'d a method in src/script/standard.cpp which we had made globally
visible for some historical reason, but which I noticed diffing that file against
upstream.
FIXME: we disable standardness checks for the non-PAK node in the PAK tests.
This is because of a bug in Elements which causes non-PAK nodes to reject
pegouts for standardness reasons. Need to fix it after the rebase.
21fa0a44ab [docs] use consistent naming for possible_overwrite (John Newbery)
2685c214cc [tests] small whitespace fixup (John Newbery)
e9936966c0 scripted-diff: Rename PRUNED to SPENT in coins tests (John Newbery)
c205979031 [docs] Improve commenting in coins.cpp|h (John Newbery)
Pull request description:
- Add full commenting for spentness / DIRTYness / FRESHness and which combinations are valid
- Remove the 'pruned' terminology, which doesn't make sense since per-txout chainstate db was merged (#10195).
- Rename `potential_overwrite` to `possible_overwrite` to standardize terminology (there were previously examples of both, which made searching the codebase difficult).
- Make other minor improvements to the comments
ACKs for top commit:
jonatack:
Re-ACK 21fa0a4 per `git diff 98bee55 21fa0a4` the only change since my previous review is the following code commenting diff in `src/coins.cpp::L177-179`; rebuilt/ran unit tests anyway as a sanity check on the unit test changes.
Tree-SHA512: 391e01588ef5edb417250080cec17361f982c4454bc5f8c6d78bbd528c68a2bb94373297760691295c24660ce1022ad3ef7599762f736c8eed772ce096d38c3d
