Merge #12639: Reduce cs_main lock and avoid extra lookups of mapAddressBook in listunspent RPC
ClosedPublic
Actions

Authored by nakihito on Nov 22 2019, 23:24.

Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGINGb03fb98a32d9: Merge #12639: Reduce cs_main lock and avoid extra lookups of mapAddressBook in…
rABCb03fb98a32d9: Merge #12639: Reduce cs_main lock and avoid extra lookups of mapAddressBook in…

Summary

Reduce cs_main lock in listunspent and refactor replacing three mapAddressBook lookups with one.

Backport of Core PR12639
https://github.com/bitcoin/bitcoin/pull/12639/

Test Plan

../configure --enable-debug
make check
test_runner.py

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

nakihito created this revision.Nov 22 2019, 23:24

Owners added a reviewer: Restricted Owners Package.Nov 22 2019, 23:24

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 22 2019, 23:24

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8240: Diff 14340.Nov 22 2019, 23:34

Where is the rest of it?

In D4516#109322, @deadalnix wrote:

Where is the rest of it?

The lock scope change currently conflicts with the AssertLockHeld(cs_main) added in D2980. I'm currently investing where the lock is not properly being applied. I figured I could do the two changes separately since they are pretty much independent changes.

Edit: D4542 actually shows the call site where cs_main is not being locked.

nakihito planned changes to this revision.Nov 26 2019, 16:49

nakihito retitled this revision from refactor: Avoid extra lookups of mapAddressBook in listunspent RPC to Merge #12639: Reduce cs_main lock and avoid extra lookups of mapAddressBook in listunspent RPC.Nov 26 2019, 17:31

nakihito edited the summary of this revision. (Show Details)

nakihito edited the test plan for this revision. (Show Details)

Added lock scope commit.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8295: Diff 14433.Nov 26 2019, 17:44

deadalnix accepted this revision.Nov 26 2019, 23:13

This revision is now accepted and ready to land.Nov 26 2019, 23:13

Closed by commit rABCb03fb98a32d9: Merge #12639: Reduce cs_main lock and avoid extra lookups of mapAddressBook in… (authored by João Barbosa <joao.paulo.barbosa@gmail.com>, committed by nakihito). · Explain WhyNov 26 2019, 23:24

This revision was automatically updated to reflect the committed changes.

nakihito added a commit: rABCb03fb98a32d9: Merge #12639: Reduce cs_main lock and avoid extra lookups of mapAddressBook in….

nakihito added a commit: rSTAGINGb03fb98a32d9: Merge #12639: Reduce cs_main lock and avoid extra lookups of mapAddressBook in….Apr 7 2020, 21:52