feat: concurrent deliverTx #53
Conversation
| Data: data, | ||
| Log: strings.TrimSpace(msgLogs.String()), | ||
| Events: events, | ||
| return &sdk.MsgsResult{ |
There was a problem hiding this comment.
fyi: Is there a reason why we need this new type? Based on the code, it looks almost identical to the existing Result. Since we have to keep merging code from origin, I wish that refactoring without functional changes should be avoided and minimal modification should be adhered to.
There was a problem hiding this comment.
It's a part of optimization. The cost of marshaling runMsg result is fairly high. Please note that now it marshals the result concurrently.
* solve a TODO in wasm's cli_test * fix how to compare hash
|
I would expect this implementation to introduce non-determimism and apphash failures. I expect that you need ReadWrite lists or some other mechanism to prevent transactions from concurrently accessing. In prior design work Cosmos contributors have thoughts after putting access lists natively in the transaction format. Now some applications might want to have dynamic runtime access patterns so it would necessary to have optional support for ReadWrite lists which implies that only part of a block will contain transactions that can be processed in parallel. |
|
Thank you for the great comment. This seems to be the first outside comment since opening our project, and we welcome it. This PR was for an experimental branch to determine whether performance was improved or not. The Thank you so much! |
Related with: https://github.com/line/link/issues/1170, Finschia/ostracon#170
Description
To optimize performance, we need to increase concurrency. After implementing concurrent checkTx(#49) and recheckTx(#52), I'd like to implement concurrent deliverTx.
Motivation and context
How has this been tested?
Checklist: