A last bunch of txqueue performance optimizations #9024

tomusdrw · 2018-07-02T12:33:25Z

Nonce cache size dependent on the pool size. Currently it was hardcoded to 4k entires, now it depends on the size of the pool (but never less than 4k). I've noticed that the cache was very often saturated when we run with large pool size.
Optimize order of cull and update_sealing in miner::chain_new_blocks. Since cull can take significant amount of time (seconds, for large pools) it's better to first attempt to construct pending block. We only need couple (hundreds) of best transactions and Readiness will make sure that they are not stale anyway. After pending block is created we attempt to clean up the pool. This should allow miners to start mining on the new block much earlier and reduce orphan rate because of that.
Split cull into chunks. To prevent acquiring write() lock of the queue for long time we are cleaning up the queue in chunks (1024 senders each chunk). This allows other threads to jump in between.

andresilva · 2018-07-02T16:23:37Z

miner/src/pool/queue.rs

-
-		let removed = self.pool.write().cull(None, state_readiness);
+		let mut removed = 0;
+		let senders: Vec<Address> = {


Shouldn't this be a set? Otherwise we may cull the same sender more than once.

We need the senders to be ordered by priority (so that we cull best senders first - more probability that it will have any effect on the queue), best_transactions in the pool (That we take the senders from) contains exactly one transaction per sender.

Is it worth adding a comment here to that effect?

I didn't realize best_transactions only had the best tx from each sender 👍.

andresilva · 2018-07-02T16:25:21Z

ethcore/src/miner/pool_client.rs

-
-const MAX_NONCE_CACHE_SIZE: usize = 4096;
-const EXPECTED_NONCE_CACHE_SIZE: usize = 2048;
+type NoncesCache = (RwLock<HashMap<Address, U256>>, usize);


We should probably create a struct for this at some point.

ascjones · 2018-07-03T09:39:17Z

Needs a merge/rebase from master

tomusdrw · 2018-07-03T12:00:40Z

@ascjones Rebased.

andresilva · 2018-07-04T15:39:09Z

Needs another rebase now that #9026 was merged.

ascjones

Code looks good. How have the efficacy of these optimisations been measured? Presumably these are the ones you were testing with Fred's node on mainnet?

tomusdrw · 2018-07-05T08:22:41Z

@ascjones yes, I was running the node and observing 3 things:

The average CPU load on the server
The peer count and sudden drops
The perf=trace logs and long pauses.

All three things are obviously related to each other, long pauses are usually caused by high CPU load in some place, which in turn leads to peer timeouts and drops. But overall chunkig the senders causes each chunk to be processed in roughly 200ms, and in between we can easily import incoming transactions or propagate the pending ones.

ascjones · 2018-07-05T09:41:54Z

Thanks @tomusdrw. I see that node has now taken the top spot!

Build needs fixing though.

ascjones

Looks good. I fixed the compilation error but the tests are failing now on the merged wasm test.

niklasad1 · 2018-07-05T13:40:02Z

@tomusdrw https://github.com/paritytech/parity/blob/832d171b2e8dc4257a9e5fe86f15c37e30f25734/ethcore/src/miner/pool_client.rs#L55-#L69

Duplicate method get

ascjones · 2018-07-05T14:11:02Z

@niklasad1 caused by me hijacking @tomusdrw 's branch to fix the build (see above).

tomusdrw · 2018-07-05T14:31:18Z

No worries, it's my fault. I've didn't try to compile after merge.

* Clear cache only when block is enacted. * Add tracing for cull. * Cull split. * Cull after creating pending block. * Add constant, remove sync::read tracing. * Reset debug. * Remove excessive tracing. * Use struct for NonceCache. * Fix build * Remove warnings. * Fix build again.

* parity-version: bump beta to 1.11.6 * scripts: remove md5 checksums (#8884) * Add support for --chain tobalaba * Convert indents to tabs :) * Fixes for misbehavior reporting in AuthorityRound (#8998) * aura: only report after checking for repeated skipped primaries * aura: refactor duplicate code for getting epoch validator set * aura: verify_external: report on validator set contract instance * aura: use correct validator set epoch number when reporting * aura: use epoch set when verifying blocks * aura: report skipped primaries when generating seal * aura: handle immediate transitions * aura: don't report skipped steps from genesis to first block * aura: fix reporting test * aura: refactor duplicate code to handle immediate_transitions * aura: let reporting fail on verify_block_basic * aura: add comment about possible failure of reporting * Only return error log for rustls (#9025) * Transaction Pool improvements (#8470) * Don't use ethereum_types in transaction pool. * Hide internal insertion_id. * Fix tests. * Review grumbles. * Improve should_replace on NonceAndGasPrice (#8980) * Additional tests for NonceAndGasPrice::should_replace. * Fix should_replace in the distinct sender case. * Use natural priority ordering to simplify should_replace. * Minimal effective gas price in the queue (#8934) * Minimal effective gas price. * Fix naming, add test * Fix minimal entry score and add test. * Fix worst_transaction. * Remove effective gas price threshold. * Don't leak gas_price decisions out of Scoring. * Never drop local transactions from different senders. (#9002) * Recently rejected cache for transaction queue (#9005) * Store recently rejected transactions. * Don't cache AlreadyImported rejections. * Make the size of transaction verification queue dependent on pool size. * Add a test for recently rejected. * Fix logging for recently rejected. * Make rejection cache smaller. * obsolete test removed * obsolete test removed * Construct cache with_capacity. * Optimize pending transactions filter (#9026) * rpc: return unordered transactions in pending transactions filter * ethcore: use LruCache for nonce cache Only clear the nonce cache when a block is retracted * Revert "ethcore: use LruCache for nonce cache" This reverts commit b382c19. * Use only cached nonces when computing pending hashes. * Give filters their own locks, so that they don't block one another. * Fix pending transaction count if not sealing. * Clear cache only when block is enacted. * Fix RPC tests. * Address review comments. * A last bunch of txqueue performance optimizations (#9024) * Clear cache only when block is enacted. * Add tracing for cull. * Cull split. * Cull after creating pending block. * Add constant, remove sync::read tracing. * Reset debug. * Remove excessive tracing. * Use struct for NonceCache. * Fix build * Remove warnings. * Fix build again. * miner: add missing macro use for trace_time * ci: remove md5 merge leftovers

…rp_sync_on_light_client * 'master' of https://github.com/paritytech/parity: Fixes for misbehavior reporting in AuthorityRound (openethereum#8998) A last bunch of txqueue performance optimizations (openethereum#9024) reduce number of constraints for triedb types (openethereum#9043) bump fs-swap to 0.2.3 so it is compatible with osx 10.11 again (openethereum#9050) Recursive test (openethereum#9042) Introduce more optional features in ethcore (openethereum#9020) Update ETSC bootnodes (openethereum#9038) Optimize pending transactions filter (openethereum#9026) eip160/eip161 spec: u64 -> BlockNumber (openethereum#9044)

tomusdrw added 7 commits July 2, 2018 14:20

Clear cache only when block is enacted.

6e5ddbe

Add tracing for cull.

729ebe5

Cull split.

8c5af99

Cull after creating pending block.

515a0cb

Add constant, remove sync::read tracing.

c4e23a6

Reset debug.

635d3f5

Remove excessive tracing.

93ad8b5

tomusdrw added A0-pleasereview 🤓 Pull request needs code review. B0-patchthis M4-core ⛓ Core client code / Rust. labels Jul 2, 2018

tomusdrw changed the title ~~A last bunch of transaction queue performance optimizations~~ A last bunch of txqueue performance optimizations Jul 2, 2018

5chdn added this to the 1.12 milestone Jul 2, 2018

5chdn requested review from ascjones and andresilva July 2, 2018 13:07

5chdn mentioned this pull request Jul 2, 2018

Beta: 1.11.6 backports #9015

Merged

12 tasks

andresilva reviewed Jul 2, 2018

View reviewed changes

tomusdrw added 2 commits July 3, 2018 13:38

Merge branch 'master' into td-cull-perf2

802124c

Use struct for NonceCache.

a2892e2

ascjones reviewed Jul 4, 2018

View reviewed changes

5chdn added the B9-blocker 🚧 This pull request blocks the next release from happening. Use only in extreme cases. label Jul 5, 2018

Merge branch 'master' into td-cull-perf2

b2df551

Fix build

326330e

ascjones approved these changes Jul 5, 2018

View reviewed changes

ascjones added the A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. label Jul 5, 2018

ascjones removed the A0-pleasereview 🤓 Pull request needs code review. label Jul 5, 2018

andresilva approved these changes Jul 5, 2018

View reviewed changes

5chdn added the P2-asap 🌊 No need to stop dead in your tracks, however issue should be addressed as soon as possible. label Jul 5, 2018

Remove warnings.

832d171

Fix build again.

d3e7cc1

andresilva merged commit aa67bd5 into master Jul 5, 2018

debris deleted the td-cull-perf2 branch July 5, 2018 15:30

tomusdrw mentioned this pull request Jul 11, 2018

Parity 1.11.6 is rejecting transactions from local accounts #9087

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A last bunch of txqueue performance optimizations #9024

A last bunch of txqueue performance optimizations #9024

tomusdrw commented Jul 2, 2018

andresilva Jul 2, 2018

tomusdrw Jul 3, 2018

ascjones Jul 4, 2018

andresilva Jul 4, 2018

andresilva Jul 2, 2018

tomusdrw Jul 3, 2018

ascjones commented Jul 3, 2018

tomusdrw commented Jul 3, 2018

andresilva commented Jul 4, 2018

ascjones left a comment

tomusdrw commented Jul 5, 2018

ascjones commented Jul 5, 2018

ascjones left a comment

niklasad1 commented Jul 5, 2018

ascjones commented Jul 5, 2018

tomusdrw commented Jul 5, 2018

A last bunch of txqueue performance optimizations #9024

A last bunch of txqueue performance optimizations #9024

Conversation

tomusdrw commented Jul 2, 2018

andresilva Jul 2, 2018

Choose a reason for hiding this comment

tomusdrw Jul 3, 2018

Choose a reason for hiding this comment

ascjones Jul 4, 2018

Choose a reason for hiding this comment

andresilva Jul 4, 2018

Choose a reason for hiding this comment

andresilva Jul 2, 2018

Choose a reason for hiding this comment

tomusdrw Jul 3, 2018

Choose a reason for hiding this comment

ascjones commented Jul 3, 2018

tomusdrw commented Jul 3, 2018

andresilva commented Jul 4, 2018

ascjones left a comment

Choose a reason for hiding this comment

tomusdrw commented Jul 5, 2018

ascjones commented Jul 5, 2018

ascjones left a comment

Choose a reason for hiding this comment

niklasad1 commented Jul 5, 2018

ascjones commented Jul 5, 2018

tomusdrw commented Jul 5, 2018