Ptrus/benchmark/runtime scheduler #3434
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ptrus
force-pushed
the
ptrus/benchmark/runtime-scheduler
branch
3 times, most recently
from
7d21467
to
27045d9
Compare
Codecov Report
@@ Coverage Diff @@
## master #3434 +/- ##
==========================================
- Coverage 66.12% 66.07% -0.06%
==========================================
Files 371 371
Lines 33368 33399 +31
==========================================
+ Hits 22065 22067 +2
- Misses 8059 8098 +39
+ Partials 3244 3234 -10
Continue to review full report at Codecov.
|
ptrus
requested review from
kostko,
peterjgilbert,
pro-wh and
Yawning
as code owners
kostko
reviewed
Oct 20, 2020
ptrus
force-pushed
the
ptrus/benchmark/runtime-scheduler
branch
4 times, most recently
from
90147e1
to
a99f9c5
Compare
kostko
approved these changes
Oct 21, 2020
| // Name is the transaction pool implementation name. | ||
| Name() string | ||
|
|
||
| // Add adds a single transaction into the transaction. |
There was a problem hiding this comment.
Suggested change
| // Add adds a single transaction into the transaction. | |
| // Add adds a single transaction into the transaction pool. |
| // RemoveBatch removes a batch from the transaction pool. | ||
| RemoveBatch(batch [][]byte) error | ||
|
|
||
| // IsQueued returns whether a call is in the queue already. |
There was a problem hiding this comment.
Suggested change
| // IsQueued returns whether a call is in the queue already. | |
| // IsQueued returns whether a transaction is in the queue already. |
| // IsQueued returns whether a call is in the queue already. | ||
| IsQueued(txHash hash.Hash) bool | ||
|
|
||
| // Size returns size of the transaction pool. |
There was a problem hiding this comment.
Suggested change
| // Size returns size of the transaction pool. | |
| // Size returns the number of transactions in the transaction pool. |
| @@ -565,8 +565,8 @@ func (n *Node) HandleNewBlockLocked(blk *block.Block) { | |||
| } | |||
| } | |||
|
|
|||
| // checkTx checks the given call in the node's runtime. | |||
| func (n *Node) checkTx(ctx context.Context, call []byte) error { | |||
| // checkTx checks the given tx in the node's runtime. | |||
There was a problem hiding this comment.
Suggested change
| // checkTx checks the given tx in the node's runtime. | |
| // checkTx requests the runtime to check the validity of the given transaction. |
ptrus
force-pushed
the
ptrus/benchmark/runtime-scheduler
branch
from
a99f9c5
to
63fbd09
Compare
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3358
Done:
txpoolinterfacemap,ordered_maptxpool implementationstxpoolandsimple schedulerordered_mapBenchmark results:
As expected in a scenario where a lot (compared to batch size) of transactions are submitted concurrently, the current queue implementation performs an order of magnitude worse, due to the removes doing
O(queueSize)operations, (which happens after each dispatch). Themapbacked tx-pool serves as a baseline implementation as it doesn't maintain transaction order. Ordered map implementation performance is comparable to the baseline map implementation, and also maintains the transactions order.The delay benchmarks however show that the overall impact of the queue implementation is not as drastic whenever the dispatch is not instant.
Based on benchmark results I think it's reasonable to switch to the ordered queue implementation.