Skip to content
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

Initial nested cache Context StateDB implementation #33

Closed
wants to merge 52 commits into from

Conversation

drklee3
Copy link
Member

@drklee3 drklee3 commented Jan 20, 2024

Description

  • Use CacheContext for StateDB snapshots
  • Update tests & benchmarks to use actual x/evm keeper instead of mock keeper

This will be later moved to the Kava repo when it's fully de-coupled from x/evm

Benchmarks

                                    │  journal.txt  │              nested-ctx.txt              │
                                    │    sec/op     │     sec/op       vs base                   │
TokenTransfer-12                       128.5µ ±  1%      167.6µ ±  2%     +30.44% (p=0.000 n=10)
EmitLogs-12                            3.128m ±  1%      3.247m ±  1%      +3.79% (p=0.000 n=10)
TokenTransferFrom-12                   127.4µ ±  0%      192.0µ ±  1%     +50.74% (p=0.000 n=10)
TokenMint-12                           123.1µ ±  1%      161.2µ ±  1%     +31.03% (p=0.000 n=10)
MessageCall-12                         49.50m ±  0%     384.17m ±  2%    +676.11% (p=0.000 n=10)
SetParams-12                           1.449µ ±  1%      1.480µ ±  1%      +2.14% (p=0.000 n=10)
GetParams-12                           2.309µ ±  0%      2.350µ ±  1%      +1.80% (p=0.000 n=10)
ApplyTransaction-12                    68.69µ ±  1%      85.50µ ±  1%     +24.47% (p=0.000 n=10)
ApplyTransactionWithLegacyTx-12        69.42µ ±  1%      86.44µ ±  1%     +24.52% (p=0.000 n=10)
ApplyTransactionWithDynamicFeeTx-12    69.34µ ±  0%      85.70µ ±  1%     +23.60% (p=0.000 n=10)
ApplyMessage-12                        15.48µ ±  0%      34.52µ ±  1%    +122.96% (p=0.000 n=10)
ApplyMessageWithLegacyTx-12            15.56µ ±  0%      34.59µ ±  0%    +122.25% (p=0.000 n=10)
ApplyMessageWithDynamicFeeTx-12        15.61µ ±  1%      34.23µ ±  1%    +119.29% (p=0.000 n=10)
CreateAccountNew-12                    2.313µ ±  2%     13.758µ ±  2%    +494.68% (p=0.000 n=10)
CreateAccountExisting-12               168.2n ±  4%    10293.0n ±  1%   +6021.32% (p=0.000 n=10)
AddBalance-12                          153.2n ±  2%    23670.0n ±  1%  +15350.39% (p=0.000 n=10)
SetCode-12                             565.2n ±  1%    11346.0n ±  1%   +1907.61% (p=0.000 n=10)
SetState-12                            565.9n ±  1%    11400.0n ±  2%   +1914.67% (p=0.000 n=10)
AddLog-12                             107.30n ± 13%      82.33n ±  7%     -23.28% (p=0.000 n=10)
Snapshot-12                            968.4n ±  0%     9351.5n ±  3%    +865.62% (p=0.000 n=10)
SubBalance-12                          155.2n ±  2%    27699.0n ±  1%  +17753.05% (p=0.000 n=10)
SetNonce-12                            60.87n ±  6%   10371.50n ±  2%  +16937.37% (p=0.000 n=10)
AddRefund-12                          30.010n ± 12%      4.748n ± 27%     -84.18% (p=0.000 n=10)
Suicide-12                             643.4n ±  4%    72174.0n ±  1%  +11118.47% (p=0.000 n=10)
geomean                                5.448µ            26.82µ          +392.20%

x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
x/evm/statedb/statedb.go Fixed Show fixed Hide fixed
@nddeluca
Copy link
Member

nddeluca commented Feb 6, 2024

Benchmarks are quite a bit slower -- I assume some of those now hit keeper methods instead of appending to journal/adding a map entry? My guess is some of those benchmarks become a little closer if the time taken in Commit is considered.

Could we design a benchmark that snapshots at various depths (maybe 10, 1000, 10000) with some changes at each and calls Commit at end? Be useful to get a better measure of performance penalty of nested cache contexts

@drklee3
Copy link
Member Author

drklee3 commented Feb 7, 2024

A lot slower at high number of layers with this benchmark:

func BenchmarkNestedSnapshot(b *testing.B) {
benches := []int{1, 4, 10, 100, 1000, 10000}
for _, layers := range benches {
b.Run(fmt.Sprintf("%d layers", layers), func(b *testing.B) {
suite := GetTestSuite(b)
for i := 0; i < b.N; i++ {
b.StopTimer()
db := statedb.New(suite.Ctx, suite.App.EvmKeeper, emptyTxConfig)
// Create layers of nested snapshots
for i := 0; i < layers; i++ {
db.Snapshot()
// Some state change each snapshot
key := common.BigToHash(big.NewInt(int64(i + 1)))
value := common.BigToHash(big.NewInt(int64(i + 1)))
db.SetState(address, key, value)
}
b.StartTimer()
require.NoError(b, db.Commit())
}
})
}
}

Benchmark takes forever to run since the setup with paused bench timer is much longer than the actual timed Commit() part

goos: darwin
goarch: arm64
pkg: github.com/evmos/ethermint/x/evm/statedb
                               │ BenchmarkNestedSnapshot-journal.txt │          BenchmarkNestedSnapshot-ctx.txt           │
                               │               sec/op                │       sec/op         vs base                       │
NestedSnapshot/1_layers-12                              556.3n ± ∞ ¹         5375.0n ± ∞ ¹       +866.21% (p=0.008 n=5)
NestedSnapshot/4_layers-12                              534.2n ± ∞ ¹        12952.0n ± ∞ ¹      +2324.56% (p=0.008 n=5)
NestedSnapshot/10_layers-12                             568.9n ± ∞ ¹        34901.0n ± ∞ ¹      +6034.82% (p=0.008 n=5)
NestedSnapshot/100_layers-12                            621.3n ± ∞ ¹      1540187.0n ± ∞ ¹    +247797.47% (p=0.008 n=5)
NestedSnapshot/1000_layers-12                           776.4n ± ∞ ¹    150212761.0n ± ∞ ¹  +19347241.71% (p=0.008 n=5)
NestedSnapshot/10000_layers-12                          1.414µ ± ∞ ¹   17185876.541µ ± ∞ ¹              ~ (p=0.095 n=2+5)
geomean                                                 697.6n                1.459m          +209092.25%
¹ need >= 6 samples for confidence interval at level 0.95

                               │ BenchmarkNestedSnapshot-journal.txt │           BenchmarkNestedSnapshot-ctx.txt            │
                               │                B/op                 │         B/op          vs base                        │
NestedSnapshot/1_layers-12                               25.00 ± ∞ ¹          6619.00 ± ∞ ¹      +26376.00% (p=0.008 n=5)
NestedSnapshot/4_layers-12                               24.00 ± ∞ ¹         17025.00 ± ∞ ¹      +70837.50% (p=0.008 n=5)
NestedSnapshot/10_layers-12                              25.00 ± ∞ ¹         43000.00 ± ∞ ¹     +171900.00% (p=0.008 n=5)
NestedSnapshot/100_layers-12                             25.00 ± ∞ ¹       1694623.00 ± ∞ ¹    +6778392.00% (p=0.008 n=5)
NestedSnapshot/1000_layers-12                            25.00 ± ∞ ¹     149449860.00 ± ∞ ¹  +597799340.00% (p=0.008 n=5)
NestedSnapshot/10000_layers-12                           30.00 ± ∞ ¹   14602752152.00 ± ∞ ¹               ~ (p=0.095 n=2+5)
geomean                                                  25.60                1.543Mi          +6319793.96%
¹ need >= 6 samples for confidence interval at level 0.95

                               │ BenchmarkNestedSnapshot-journal.txt │           BenchmarkNestedSnapshot-ctx.txt           │
                               │              allocs/op              │      allocs/op       vs base                        │
NestedSnapshot/1_layers-12                               1.000 ± ∞ ¹          98.000 ± ∞ ¹       +9700.00% (p=0.008 n=5)
NestedSnapshot/4_layers-12                               1.000 ± ∞ ¹         260.000 ± ∞ ¹      +25900.00% (p=0.008 n=5)
NestedSnapshot/10_layers-12                              1.000 ± ∞ ¹         644.000 ± ∞ ¹      +64300.00% (p=0.008 n=5)
NestedSnapshot/100_layers-12                             1.000 ± ∞ ¹       15990.000 ± ∞ ¹    +1598900.00% (p=0.008 n=5)
NestedSnapshot/1000_layers-12                            1.000 ± ∞ ¹     1071292.000 ± ∞ ¹  +107129100.00% (p=0.008 n=5)
NestedSnapshot/10000_layers-12                           1.000 ± ∞ ¹   101818490.000 ± ∞ ¹               ~ (p=0.095 n=2+5)
geomean                                                  1.000                17.49k          +1748852.62%
¹ need >= 6 samples for confidence interval at level 0.95

@evgeniy-scherbina
Copy link

LGTM

@drklee3 drklee3 changed the base branch from dl-decouple-statedb to kava/release/v0.24.x February 9, 2024 22:49
Previous to this change, keeper SetState() only deletes when the
len(value) == 0. However, since value in the StateDB is a common.Hash{},
this will always be the HashLength. Deleting the value when the empty
hash is set will also ensure specific no-op changes will not be committed,
e.g. Creating a state, then deleting it in the same tx will result in
no state added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants