Merge #12330: Reduce scope of cs_wallet locks in listtransactions
ClosedPublic
Actions

Authored by nakihito on May 29 2020, 22:38.

Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rABC4b993b82a11c: Merge #12330: Reduce scope of cs_wallet locks in listtransactions

Summary

c409b1adac [rpc] Reduce scope of cs_main and cs_wallet locks in listtransactions (João Barbosa)

Pull request description:

Trivial change, no behaviour change.

Benchmark done as follow:
 - run with `-regtest`
 - wallet with 5000 transactions
 - measured the time spent with the lock and the total time
 - times are an average of 100 `listtransactions --count=...` calls

| `--count` | lock (ms) | total (ms) | saving |
|--:|--:|--:|--:|
| 10 | 0.2230 | 0.2510 | 11% |
| 100 | 2.5150 | 2.8690 | 12% |
| 1000 | 20.0320 | 23.3490 | 14% |
| 10000 | 105.2070 | 125.5310 | 16% |

Tree-SHA512: ebedfeeb4c8ad75c89128e53cae976a82967dbb5ffd129da0f7204ccf9c3c15070b3d509f3767bebd745512e410200cc546147c836e82409f95fc9b8d14fc3ed

Backport of Core PR12330

Reviewer note: the change might be easier to see with w=1.

Test Plan

cmake -DCMAKE_BUILD_TYPE=Debug -GNinja ..
ninja
ninja check
ninja check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

nakihito created this revision.May 29 2020, 22:38

Owners added a reviewer: Restricted Owners Package.May 29 2020, 22:38

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 29 2020, 22:38

nakihito requested review of this revision.May 29 2020, 22:38

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

nakihito planned changes to this revision.May 29 2020, 22:39

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) started.

Build Bitcoin ABC Diffs / Diff Testing (build-werror) started.

nakihito retitled this revision from Merge #12330: Reduce scope of cs_main and cs_wallet locks in listtransactions to Merge #12330: Reduce scope of cs_wallet locks in listtransactions.May 29 2020, 22:42

Build Bitcoin ABC Diffs / Diff Testing (build-werror) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) started.

Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) passed.

Build Bitcoin ABC Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B11097: Diff 20723.May 29 2020, 22:52

nakihito requested review of this revision.May 29 2020, 23:28

nakihito added inline comments.

src/wallet/rpcwallet.cpp
1698 ↗	(On Diff #20723)	https://reviews.bitcoinabc.org/D5075#change-4q6XVS3POqj5 was done out-of-order.
1732 ↗	(On Diff #20723)	This should be the result if the backport order had been preserved: https://github.com/bitcoin/bitcoin/pull/14437/commits/79d579f4e11b57f90fed314bccd25230f918729f?file-filters%5B%5D=.cpp#diff-df7d84ff2f53fcb2a0dc15a3a51e55ceL1409-L1410