Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

stateDB txlog store enhancement #461

Merged
merged 18 commits into from
Aug 31, 2021
Merged

Conversation

JayT106
Copy link
Contributor

@JayT106 JayT106 commented Aug 19, 2021

Description

after profiling BenchmarkEmitLogs in #436
found we can improve the emit log speed by changing the log store structure.

Therefore, we store each log separately prefixlogkey | txHash | log.Index, not stores by []logs

The change improves the log store by around 90x within the testcase.

In #436:

goos: darwin
goarch: amd64
pkg: github.com/tharsis/ethermint/x/evm/keeper
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkEmitLogs-12    	       5	1197719670 ns/op

In This PR:

goos: darwin
goarch: amd64
pkg: github.com/tharsis/ethermint/x/evm/keeper
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkEmitLogs-12    	     453	  13216579 ns/op

Updated PR:

pkg: github.com/tharsis/ethermint/x/evm/keeper
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkEmitLogs
BenchmarkEmitLogs-12         	      97	  12626105 ns/op

For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@JayT106 JayT106 changed the title stateDB log store enhancement stateDB txlog store enhancement Aug 19, 2021
@codecov
Copy link

codecov bot commented Aug 19, 2021

Codecov Report

Merging #461 (443dffc) into main (7b3ceec) will increase coverage by 0.03%.
The diff coverage is 57.69%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #461      +/-   ##
==========================================
+ Coverage   49.18%   49.22%   +0.03%     
==========================================
  Files          57       57              
  Lines        5363     5408      +45     
==========================================
+ Hits         2638     2662      +24     
- Misses       2608     2629      +21     
  Partials      117      117              
Impacted Files Coverage Δ
x/evm/keeper/keeper.go 66.35% <46.77%> (-4.34%) ⬇️
x/evm/keeper/grpc_query.go 71.21% <100.00%> (+0.80%) ⬆️
x/evm/keeper/statedb.go 84.61% <100.00%> (-0.16%) ⬇️

x/evm/keeper/statedb.go Outdated Show resolved Hide resolved
x/evm/keeper/keeper.go Outdated Show resolved Hide resolved
x/evm/keeper/keeper.go Outdated Show resolved Hide resolved
x/evm/keeper/keeper.go Outdated Show resolved Hide resolved
x/evm/types/key.go Outdated Show resolved Hide resolved
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment. I would like to propose a cleaner implementation that doesn't add a new store key and relies on the store Iterator to retrieve the logs

x/evm/keeper/keeper.go Outdated Show resolved Hide resolved
x/evm/types/key.go Outdated Show resolved Hide resolved
@orijbot
Copy link

orijbot commented Aug 27, 2021

x/evm/keeper/keeper.go Outdated Show resolved Hide resolved
x/evm/keeper/keeper.go Show resolved Hide resolved
x/evm/keeper/keeper.go Outdated Show resolved Hide resolved
x/evm/keeper/keeper.go Show resolved Hide resolved
x/evm/keeper/keeper.go Show resolved Hide resolved
x/evm/keeper/statedb.go Outdated Show resolved Hide resolved
x/evm/keeper/keeper.go Show resolved Hide resolved
x/evm/keeper/grpc_query.go Outdated Show resolved Hide resolved
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Thanks @JayT106! Can you add a Changelog entry under the improvements section 🙏

@fedekunze fedekunze added the C:x/evm EVM module label Aug 28, 2021
@JayT106
Copy link
Contributor Author

JayT106 commented Aug 30, 2021

ACK. Thanks @JayT106! Can you add a Changelog entry under the improvements section 🙏

Sure, will do. Thanks for the review.

CHANGELOG.md Outdated Show resolved Hide resolved
@fedekunze fedekunze enabled auto-merge (squash) August 30, 2021 14:51
@fedekunze fedekunze added the automerge Automatically merge PR once all prerequisites pass. label Aug 31, 2021
@fedekunze fedekunze merged commit 9a8827e into evmos:main Aug 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Automatically merge PR once all prerequisites pass. C:x/evm EVM module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants