Pass mempool pointer to UnloadBlockIndex/GetCoinsCacheSizeState #19604

pull MarcoFalke wants to merge 3 commits into bitcoin:master from MarcoFalke:2007-memPointer changing 8 files +27 −36

MarcoFalke commented at 5:52 AM on July 28, 2020: member

Split out from #19556

Instead of relying on the implicit mempool global, pass a mempool pointer (which can be 0). This helps with testing, code clarity and unlocks the features described in #19556.
MarcoFalke added the label Refactoring on Jul 28, 2020
MarcoFalke added the label Validation on Jul 28, 2020
MarcoFalke force-pushed on Jul 28, 2020
jamesob commented at 2:33 PM on July 28, 2020: member

ACK https://github.com/bitcoin/bitcoin/pull/19604/commits/faabb459fe168d02a5e587000ebfad46b06bf7cc

Will test locally soon.
MarcoFalke cross-referenced this on Jul 28, 2020 from issue Remove mempool global by MarcoFalke
DrahtBot commented at 5:07 PM on July 28, 2020: contributor


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #19306 (refactor: Replace RecursiveMutex with Mutex in CTxMemPool by hebasto)
- #19271 (validation: Warm coins cache during prevalidation to connect blocks faster by andrewtoth)
- #18766 (Disable fee estimation in blocksonly mode by darosior)
- #18731 (refactor: Make CCheckQueue RAII-styled by hebasto)
- #18710 (Add local thread pool to CCheckQueue by hebasto)
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.
darosior approved
darosior commented at 6:24 PM on July 28, 2020: member

ACK faabb459fe168d02a5e587000ebfad46b06bf7cc
DrahtBot cross-referenced this on Jul 28, 2020 from issue validation: Warm coins cache during prevalidation to connect blocks faster by andrewtoth
DrahtBot cross-referenced this on Jul 28, 2020 from issue coins: allow cache resize after init by jamesob
test: Simplify cs_main locks faec851b6e
DrahtBot added the label Needs rebase on Jul 29, 2020
MarcoFalke force-pushed on Jul 29, 2020
MarcoFalke commented at 6:14 AM on July 29, 2020: member

Rebased due to changes in adjacent line, also added unrelated test cleanup commit
DrahtBot removed the label Needs rebase on Jul 29, 2020

in src/qt/test/apptests.cpp:87 in fad551eb7e outdated

  82 | @@ -83,7 +83,8 @@ void AppTests::appTests()
  83 |      // Reset global state to avoid interfering with later tests.
  84 |      LogInstance().DisconnectTestLogger();
  85 |      AbortShutdown();
  86 | -    UnloadBlockIndex();
  87 | +    CTxMemPool dummy;
  88 | +    UnloadBlockIndex(&dummy);

jnewbery commented at 10:18 AM on July 29, 2020:

Can you pass nullptr here?

MarcoFalke commented at 10:31 AM on July 29, 2020:

Thx. Fixed

jnewbery commented at 10:20 AM on July 29, 2020: member

Code review ACK fad551eb7e712e99fb8338eb89ae963b1d12ba3e. One suggested simplification inline.
Pass mempool pointer to UnloadBlockIndex fac674db20
Pass mempool pointer to GetCoinsCacheSizeState fae8c28dae
MarcoFalke force-pushed on Jul 29, 2020
jnewbery commented at 11:44 AM on July 29, 2020: member

code review ACK fae8c28dae747f9c4c6481049742346d18202fc8
darosior approved
darosior commented at 11:45 AM on July 29, 2020: member

Tested ACK fae8c28dae747f9c4c6481049742346d18202fc8
jamesob approved

jamesob commented at 1:40 PM on July 29, 2020: member

ACK fae8c28dae747f9c4c6481049742346d18202fc8 (jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to)

Built locally and ran unittests.

<details><summary>Show platform data</summary> <p>

Tested on Linux-4.19.0-9-amd64-x86_64-with-glibc2.28

Configured with ./configure LDFLAGS=-L/home/james/src/bitcoin/db4/lib/ CPPFLAGS=-I/home/james/src/bitcoin/db4/include/ CXXFLAGS=-std=c++11 -Wthread-safety-analysis -Werror --enable-wallet --enable-debug --with-daemon CXX=/usr/bin/clang++ CC=/usr/bin/clang --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery --disable-jni --no-create --no-recursion

Compiled with /usr/bin/ccache /usr/bin/clang++ -std=c++11 -mavx -mavx2 -std=c++11 -Wthread-safety-analysis -Werror -O0 -g3 -ftrapv  -Wstack-protector -fstack-protector-all -msse4 -msha -msse4.1 -msse4.2  i

Compiler version: clang version 7.0.1-8 (tags/RELEASE_701/final)

</p></details>

<details><summary>Show signature data</summary> <p>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK fae8c28dae747f9c4c6481049742346d18202fc8 ([`jamesob/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to`](https://github.com/jamesob/bitcoin/tree/ackr/19604.1.MarcoFalke.pass_mempool_pointer_to))

Built locally and ran unittests.

<details><summary>Show platform data</summary>
<p>

Tested on Linux-4.19.0-9-amd64-x86_64-with-glibc2.28

Configured with ./configure LDFLAGS=-L/home/james/src/bitcoin/db4/lib/ CPPFLAGS=-I/home/james/src/bitcoin/db4/include/ CXXFLAGS=-std=c++11 -Wthread-safety-analysis -Werror --enable-wallet --enable-debug --with-daemon CXX=/usr/bin/clang++ CC=/usr/bin/clang --disable-shared --with-pic --enable-benchmark=no --with-bignum=no --enable-module-recovery --disable-jni --no-create --no-recursion

Compiled with /usr/bin/ccache /usr/bin/clang++ -std=c++11 -mavx -mavx2 -std=c++11 -Wthread-safety-analysis -Werror -O0 -g3 -ftrapv -Wstack-protector -fstack-protector-all -msse4 -msha -msse4.1 -msse4.2 i

Compiler version: clang version 7.0.1-8 (tags/RELEASE_701/final)


</p></details>
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEpm2+LMqPsWj2rAwWyrywS1A+5mgFAl8hfCAACgkQyrywS1A+
5mgfqg//fT5WYUt3TfjmMu+QR1mi5y8XAi+H0YQ9Jxmt3PZI7NdEwe/WNENO+s1J
nBw7Z+NI5F+u0pEB2s517aUQBVUjL0W9yyRQ3Y2m2fsObjMbH2dPR2RWn72Hheub
dis1vfJpbp3Jzz6dWQzGd5E19HNN+3oJCgZ3HQbjm9+yPuKfajQuQ4KXhly47eHZ
DDANRUHuTJITYxb8wge++xNiFaG0hTdhbBX01A5cCg20XUDx6DiCzEt61thBlYGF
1Zp5ddRCDbER1pLn7K1U0Lg1b5tVnOVu6kO0G9cMXNYnHlVOKlBupVOvKFZZKIQO
Pl8lXMc0cLUcCBWsSzkw2xLVSiRVPwaQ9bFo/zlPZUNRwxdx6kUrTPXpYS8sZmgu
qdykwMcqmyGrS9SDr5IwiZC37gfuRPNnF24cCatUQqfFWsvnrnGwkkjmINS1SPM9
slWMc7gZFQ2u+T46NsnhpmsJ8AmjZ6muaYyn+o5SQIZnWxRKNmGH8I73qKPoo1K8
B4Mu31BeFsS5s9sXjBt9DmRIceWH1YXblOJp2j4lJ55fYxAsMm5/f22k41fGlDa1
KtJ31YbXCqc1Hh4SrNlAucsmHONI7xGVs5ABrouveInj2PMascWRSZKKdk8MlD7l
YN+zp5MykAp8/eot5qdj8W9qv7+dGuh7N9V3nyNtdUZWBeyZTDk=
=zZ7M
-----END PGP SIGNATURE-----

</p></details>

darosior cross-referenced this on Jul 29, 2020 from issue Disable fee estimation in blocksonly mode (by removing the fee estimates global) by darosior
DrahtBot cross-referenced this on Jul 29, 2020 from issue refactor: Replace RecursiveMutex with Mutex in CTxMemPool by hebasto
DrahtBot cross-referenced this on Jul 29, 2020 from issue refactor: Make CCheckQueue RAII-styled by hebasto
DrahtBot cross-referenced this on Jul 29, 2020 from issue Add local thread pool to CCheckQueue by hebasto
fjahr commented at 1:48 PM on July 30, 2020: contributor

Code review ACK fae8c28dae747f9c4c6481049742346d18202fc8
MarcoFalke merged this on Jul 30, 2020
MarcoFalke closed this on Jul 30, 2020
Fabcien referenced this in commit 881afbd7aa on Jul 8, 2021
Fabcien referenced this in commit eb27969520 on Jul 8, 2021
Fabcien referenced this in commit af4f4249e6 on Jul 8, 2021
bitcoin locked this on Feb 15, 2022
MarcoFalke deleted the branch on Aug 14, 2022

Contributors

Labels

Refactoring Validation

Linked (view graph)

#18766 Disable fee estimation in blocksonly mode (by removing the fee estimates global)#19556 Remove mempool global

Pass mempool pointer to UnloadBlockIndex/GetCoinsCacheSizeState #19604

Conflicts