mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-02-26 09:09:55 +01:00
1801d8c3c9
d7f61e7d59rpc: reduce LOCK(cs_main) scope in gettxoutproof (Andrew Toth)4d92b5aabarpc: reduce LOCK(cs_main) scope in GetUndoChecked and getblockstats (Andrew Toth)efd82aec8arpc: reduce LOCK(cs_main) scope in blockToJSON (Andrew Toth)f00808e932rpc: reduce LOCK(cs_main) scope in GetBlockChecked and getblock (Andrew Toth)7d253c943fzmq: remove LOCK(cs_main) from NotifyBlock (Andrew Toth)c75e3d2772rest: reduce LOCK(cs_main) scope in rest_block (Andrew Toth) Pull request description: Picking up from #21006. After commitccd8ef65f9it is no longer required to hold `cs_main` when calling `ReadBlockFromDisk`. This can be verified in `master` at https://github.com/bitcoin/bitcoin/blob/master/src/node/blockstorage.cpp#L755. Same can be seen for `UndoReadFromDisk` https://github.com/bitcoin/bitcoin/blob/master/src/node/blockstorage.cpp#L485. The first commit moves `ReadBlockFromDisk` outside the lock scope in `rest_block`, where we can see a huge performance improvement when fetching blocks with multiple threads. My test setup, on an Intel i7 with 8 cores (16 threads): 1. Start a fully synced bitcoind, with this `bitcoin.conf`: ``` rest=1 rpcthreads=16 rpcworkqueue=64 rpcuser=user rpcpassword=password ``` 2. Run ApacheBench: 10000 requests, 16 parallel threads, fetching block nr. 750000 in binary: ``` ab -n 10000 -c 16 "http://127.0.0.1:8332/rest/block/0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e.bin" ``` Time per request (mean) 183 ms on master 30 ms this branch So this can process 6.1 times as many requests, and saturates all the cores instead of keeping them partly idle waiting in the lock. With 8 threads the mean times were 90 ms on master and 19 ms on this branch, a speedup of 4.7x. Big thanks to martinus for finding this and the original PR. The second commit is from a suggestion on the original PR by jonatack to remove the unnecessary `LOCK(cs_main)` in the zmq notifier's `NotifyBlock`. I also found that this approach could be applied to rpcs `getblock` (including `verbosity=3`), `getblockstats`, and `gettxoutproof` with similar very good results. The above benchmarks steps need to be modified slightly for RPC. Run the following ApacheBench command with different request data in a file named `data.json`: ``` ab -p data.json -n 10000 -c 16 -A user:password "http://127.0.0.1:8332/" ``` For `getblock`, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblock", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e"]} ``` master - 184 ms mean request time branch - 28 ms mean request time For `getblock` with verbosity level 3, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblock", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e", 3]} ``` This verbosity level fetches an undo file from disk, so it benefits from this approach as well. However, a lot of time is spent serializing to JSON so the performance gain is not as severe. master - 818 ms mean request time branch - 505 ms mean request time For `getblockstats`, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblockstats", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e", ["minfeerate","avgfeerate"]]} ``` This request used a lock on reading both a block and undo file, so the results are very good. master - 244 ms mean request time branch - 28 ms mean request time ACKs for top commit: MarcoFalke: re-ACKd7f61e7d59💫 hebasto: ACKd7f61e7d59, I have reviewed the code and it looks OK. Did not make benchmarking though. Tree-SHA512: 305ac945b4571c5f47646d4f0e78180d7a3d40b2f70ee43e4b3e00c96a465f6d0b9c750b8e85c89ed833e557e2cdb5896743f07ef90e4e53d4ad85452b545886