celo-org / celo-blockchain

Official repository for the golang Celo Blockchain
https://celo.org
GNU Lesser General Public License v3.0
560 stars 198 forks source link

Improve performance of answerGetBlockBodiesQuery #2198

Closed carterqw2 closed 11 months ago

carterqw2 commented 12 months ago

Description

Avoid decoding and re-encoding of the block body by using available RLP-encoded block body from the database, combine it with the RLP-encoded block hash to avoid unnecessary processing and extra memory allocations.

Screenshot 2023-06-26 at 17 36 19

Benchmarks:

Original Ethereum version (highly efficient as block body is taken either from the cache or database directly).

BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-1-8           14819121            92.65 ns/op       24 B/op          1 allocs/op
BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-10-8           1318404           896.9 ns/op       744 B/op          5 allocs/op
BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-100-8           153598          7907 ns/op        6120 B/op          8 allocs/op

Current Celo-version

BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-1-8            1000000          1278 ns/op         448 B/op         12 allocs/op
BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-10-8             81495         12543 ns/op        4984 B/op        115 allocs/op
BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-100-8            10000        115520 ns/op       48523 B/op       1108 allocs/op

Suggested version

BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-1-8            3173032           370.5 ns/op       256 B/op          6 allocs/op
BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-10-8            297216          3805 ns/op        3064 B/op         55 allocs/op
BenchmarkAnswerGetBlockBodiesQuery/GetBlockBodies-100-8            30360         37067 ns/op       29322 B/op        508 allocs/op

3x more CPU-efficient and 1.5x more memory-efficient. A larger change would be storing RLP-encoded blockBodyWithBlockHash structures instead of just block bodies but it would significantly increase the diff.

Other changes

No.

Tested

Existing tests pass, added unit tests and the benchmark.

Backwards compatibility

Yes.

github-actions[bot] commented 12 months ago

Coverage from tests in ./e2e_test/... for ./consensus/istanbul/... at commit 5c44f5dcfbff4fbbacdcddd4eb25063be47e1fbf

coverage: 50.0% of statements across all listed packages
coverage:  63.2% of statements in consensus/istanbul
coverage:  42.7% of statements in consensus/istanbul/announce
coverage:  55.8% of statements in consensus/istanbul/backend
coverage:   0.0% of statements in consensus/istanbul/backend/backendtest
coverage:  24.3% of statements in consensus/istanbul/backend/internal/replica
coverage:  62.1% of statements in consensus/istanbul/core
coverage:  50.0% of statements in consensus/istanbul/db
coverage:   0.0% of statements in consensus/istanbul/proxy
coverage:  64.2% of statements in consensus/istanbul/uptime
coverage:  51.8% of statements in consensus/istanbul/validator
coverage:  79.2% of statements in consensus/istanbul/validator/random
github-actions[bot] commented 12 months ago

5860 passed, 4 failed, 45 skipped

Test failures:
  TestTransferCELOPreGingerbread: e2e_test
Checking getExchangeSpenders. spenders = [0x000000000000000000000000000000000000d028]
Checking medianRate. numerator = 1000000000000000000000000  denominator = 1000000000000000000000000
Checking gas price minimum. cusdValue = 100000000
e2e_transfer_test.go:277: 
  TestSyncNoStorageAndOneCodeCappedPeer: snap
    sync_test.go:1105: Error, expected < 100 invocations, got 114
  TestEthClient: ethclient
INFO [10-24|09:15:19.233] Persisted trie from memory database      nodes=1 size=146.00B time="14.99µs" gcnodes=0 gcsize=0.00B gctime=0s livenodes=1 livesize=0.00B
INFO [10-24|09:15:19.237] Allocated trie memory caches             clean=0.00B dirty=0.00B
WARN [10-24|09:15:19.237] Sanitizing invalid gateway fee           provided= updated=0
INFO [10-24|09:15:19.237] Writing custom genesis block 
INFO [10-24|09:15:19.238] Persisted trie from memory database      nodes=1 size=146.00B time="5.971µs" gcnodes=0 gcsize=0.00B gctime=0s livenodes=1 livesize=0.00B
INFO [10-24|09:15:19.238] Initialised chain configuration          config="{ChainID: 1337 Homestead: 0 DAO:  DAOSupport: false EIP150: 0 EIP155: 0 EIP158: 0 Byzantium: 0 Constantinople: 0 Petersburg: 0 Istanbul: 0 Churrito: 0, Donut: 0, Espresso: 0, Gingerbread: 0, Gingerbread P2: 0, Engine: MockEngine}"
INFO [10-24|09:15:19.238] Initialising Ethereum protocol           versions=[67] network=0 dbversion=
INFO [10-24|09:15:19.239] Loaded most recent local header          number=0 hash=0f9636..27f64f td=1 age=54y7mo4d
INFO [10-24|09:15:19.239] Loaded most recent local full block      number=0 hash=0f9636..27f64f td=1 age=54y7mo4d
INFO [10-24|09:15:19.239] Loaded most recent local fast block      number=0 hash=0f9636..27f64f td=1 age=54y7mo4d
WARN [10-24|09:15:19.239] Sanitizing invalid txpool journal time   provided=0s    updated=1s
WARN [10-24|09:15:19.239] Sanitizing invalid txpool price bump     provided=0     updated=10
WARN [10-24|09:15:19.239] Sanitizing invalid txpool account slots  provided=0     updated=16
WARN [10-24|09:15:19.239] Sanitizing invalid txpool global slots   provided=0     updated=5120
WARN [10-24|09:15:19.239] Sanitizing invalid txpool account queue  provided=0     updated=64
WARN [10-24|09:15:19.239] Sanitizing invalid txpool global queue   provided=0     updated=1024
WARN [10-24|09:15:19.239] Sanitizing invalid txpool lifetime       provided=0s    updated=3h0m0s
ERROR[10-24|09:15:19.239] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.239] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.239] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.239] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.239] getWhitelist invocation failed           err="Registry not deployed"
WARN [10-24|09:15:19.240] Error reading unclean shutdown markers   error="not found"
INFO [10-24|09:15:19.240] Starting peer-to-peer node               instance=ethclient.test/linux-amd64/go1.18.10
INFO [10-24|09:15:19.247] New local node record                    seq=1,698,138,919,246 id=830899b3d559192e ip=127.0.0.1 udp=35827 tcp=0
INFO [10-24|09:15:19.247] Started P2P networking                   self="enode://ec124f45942c0d297536c7110c23715af8dacea3014e415183a0a3eaca9dcee46495196dd12218293d51cca2ecb992fb359e2459bf53c5adb483937a5c2997c6@127.0.0.1:0?discport=35827" maxdialed=0 maxinbound=0
ERROR[10-24|09:15:19.248] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.248] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.248] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.248] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.248] Error invoking evm function: can't unpack result to=0x000000000000000000000000000000000000ce10 method=getAddressFor err="abi: attempting to unmarshall an empty string while arguments are expected" maxgas=100,000
ERROR[10-24|09:15:19.248] getWhitelist invocation failed           err="Registry not deployed"
INFO [10-24|09:15:19.249] Imported new chain segment               blocks=1 txs=0 mgas=0.000 elapsed=1.181ms mgasps=0.000 number=1 hash=39401a..57383c age=54y7mo4d dirty=686.00B
  TestEthClient/TestTxInBlockInterrupted: ethclient
    ethclient_test.go:382: error should not be nil/notfound
This test report was produced by the test-summary action.  Made with ❤️ in Cambridge.
codecov[bot] commented 12 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8ceea79) 55.08% compared to head (8780433) 46.58%. Report is 1 commits behind head on master.

:exclamation: Current head 8780433 differs from pull request most recent head 62369f2. Consider uploading reports for the commit 62369f2 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2198 +/- ## ========================================== - Coverage 55.08% 46.58% -8.51% ========================================== Files 681 229 -452 Lines 114413 29830 -84583 ========================================== - Hits 63028 13895 -49133 + Misses 47496 15084 -32412 + Partials 3889 851 -3038 ``` [see 456 files with indirect coverage changes](https://app.codecov.io/gh/celo-org/celo-blockchain/pull/2198/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=celo-org)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

carterqw2 commented 12 months ago

Looks good, maybe it's worthwhile to add the benchmarking code as well?

Added.