Page MenuHomePhabricator

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

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

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nakihito created this revision.Fri, Nov 22, 23:24
Owners added a reviewer: Restricted Owners Package.Fri, Nov 22, 23:24
Herald added a reviewer: Restricted Project. · View Herald TranscriptFri, Nov 22, 23:24

Where is the rest of it?

nakihito added a comment.EditedMon, Nov 25, 19:29

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.Tue, Nov 26, 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.Tue, Nov 26, 17:31
nakihito edited the summary of this revision. (Show Details)
nakihito edited the test plan for this revision. (Show Details)
nakihito updated this revision to Diff 14433.Tue, Nov 26, 17:32

Added lock scope commit.

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