-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf: fix to use easyjson instead of amino when marshal abci logs #208
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
wetcod
force-pushed
the
sangyeop/v2/apply-easyjson
branch
from
May 26, 2021 10:46
fbd2898
to
9f17215
Compare
wetcod
force-pushed
the
sangyeop/v2/apply-easyjson
branch
from
May 26, 2021 10:46
9f17215
to
114399f
Compare
Codecov Report
@@ Coverage Diff @@
## v2/develop #208 +/- ##
==============================================
- Coverage 53.49% 53.48% -0.01%
==============================================
Files 653 652 -1
Lines 47376 47345 -31
==============================================
- Hits 25343 25324 -19
+ Misses 19178 19167 -11
+ Partials 2855 2854 -1
|
egonspace
reviewed
May 27, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
egonspace
approved these changes
May 27, 2021
96 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.
Description
I found a bottleneck in
runMsgs
, the process of converting theResult.Log
to a string.( https://github.com/line/lbm-sdk/blob/v2/develop/baseapp/baseapp.go#L767 )
When marshaling
Result.Log
with JSON, it takes a long time because many reflects are generated byamino
codec.Since
Result.Log
is not registered inamino
codec asRegisterConcrete
, it has the same format as json.So, instead of using reflects, change it to use the
MarshalJSON
function created witheasyjson
.(In the case of the types registered in
amino
codec withRegisterConcrete
,easyjson
must not be used.)Profile
AS-IS
TO-BE
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes