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

feat: Fast storage optimization for queries and iterations #468

Merged
merged 123 commits into from
Apr 9, 2022

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Feb 13, 2022

Background

Link to the original spec

Link to the original PR in Osmosis

Background
Historically IAVL has had a very slow performance during state machine execution, and for responding to queries to live state. This release speeds up these routines by an order of magnitude, alleviating large amounts of pressure from all users of the IAVL database.

Details

This PR introduces an auxiliary fast storage system to IAVL, which represents a copy of the latest state much more amenable to efficient querying and iteration.

Prior to this PR, all data gets & iterations suffered two significant performance drawdowns:

  • Every get/iteration is forced to walk the tree structure
  • Every node (including leaves) is stored on disk, with its key being a SHA256 hash of something.

All nodes were indexed by their Merkle tree inner node hash. This breaks data locality and makes every Get() that should be in RAM / CPU caches instead be a random leveldb file open.

The fast storage nodes are instead indexed by the logical key on the disk. This allows us to preserve data locality for the latest state, significantly improving iterations and queries. (Depending on the particular benchmark, between 5-30x improvements) This implementation introduces a negligible overhead for writes.

Downgrade-re-upgrade protection
We introduced a downgrade and re-upgrade protection where we guard for potential downgrades of iavl and the subsequent enablement of the fast storage again. This is done so by storing the metadata about the current version of the storage and the latest live state stored.

Summary of Changes
IAVL is divided into two trees, mutable_tree and immutable_tree. Sets only happen on the mutable tree.

Things that need to change and be investigated for getting and setting, and the fast node:

  • mutable tree

    • GetVersioned
      • Change the logic to check the FastNode cache first, then do the GetImmutable logic
    • Set
      • Cache unsaved fast nodes in memory, avoid persisting them to disk right away
      • Update unsaved removals if needed
    • Remove
      • Cache unsaved removals in memory, avoid persisting changes to disk
      • Update unsaved additions if needed
    • SaveVersion
      • Persist unsaved fast node additions and removals to disk
      • Sort by key before saving to db to ensure data locality
      • Potential optimizations:
        • Look into if removing before writing to db is faster
        • Look into if combining sorted removals with sorted additions is faster than doing them one after the other
        • Conclusion is documented here: Develop #13
    • Iterate
      • Now that we have some unsaved changes cached in the mutable tree, we cannot use the Iterate's implementation from the immutable tree. Introduce this new method.
      • Ensure that we iterate in the most efficient manner in sorted order by having 2 pointers:
        1. to the next element on disk
        2. to the next unsaved change in the mutable tree
      • Compare the values of the 2 pointers on each iteration and choose the next one
    • Iterator
      • Immutable tree is embedded into mutable. From the way composition works in Go, the mutable tree can access all methods and fields of the immutable. So we need to overwrite the implementation of the immutable tree and return an invalid iterator by default.
      • The iterator in the mutable tree is invalid because we cannot support updates and delayed iterations at the same time.
    • Get
      • For the same reason as in Iterate, we must check unsaved additions first before attempting to use the strategy employed by the immutable tree
    • enableFastStorageAndCommit and its variations
      • These are the methods that are used to perform automatic upgrades to fast storage if the system detects that fast cache is not used. This detection happens by checking the value of a special metadata key called mstorage_version where m is a new prefix. If the version is lower than the fastStorageVersionValue threshold - migration is triggered.
      • LoadVersion, LazyLoadVersion
        • added upgrade logic and downgrade + re-upgrade protection. It works by storing the storage version in the database.
        • if storage version is low -> upgrade
        • if the latest fast node version on disk does not match latest version in ndb -> upgrade
      • isUpgradeable - determines if the upgrade sis going to be performed. This method can be called on the SDK side to determine if we should log a warning message
  • immutable_tree

    • Get and (GetWithIndex
      • renamed Get to GetWithIndex. GetWithIndex always uses the default live state traversal strategy
      • introduced Get method. Get attempts to use the fast cache first. Only fallbacks to regular tree traversal strategy if the fast cache is disabled or tree is not of the latest version
    • Iterator
      • returns either regular or updated fast iterator (see more below) depending on if fast storage is enabled and migration is complete
  • nodedb

    • updated and tested the underlying storage management logic
  • fast_iterator

    • introduced and tested a new iterator that binds directly to the database iterator by searching for keys that begin with the prefix f which stands for fast. Basically, all fast nodes are sorted on disk by key in ascending order so we can simply traverse the disk ensuring efficient hardware access.
  • unsaved_fast_iterator

    • iterates over the latest state via fast nodes, taking into account unsaved changes to fast nodes that could have been made within a mutable tree
  • testing

    • unit tests for mutable tree Get, Set, Save, Delete
    • unit tests for immutable - Get, Iterator - fast and slow
    • some unit tests for nodedb
    • updated randomized tests that function like integration tests
    • updated bench tests

Benchstat

~/projects/iavl (master)$ benchstat bench_old_large.log bench_fast_large.log
name                                                             old time/op    new time/op    delta
Large/memdb-1000000-100-16-40/query-no-in-tree-guarantee-16        11.6µs ±18%     2.1µs ± 6%  -81.86%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/query-hits-16                        13.0µs ± 3%     1.5µs ±21%  -88.06%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/iteration-16                          7.05s ± 9%     0.52s ± 2%  -92.67%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/update-16                             220µs ±10%     218µs ± 9%     ~     (p=0.841 n=5+5)
Large/memdb-1000000-100-16-40/block-16                             23.0ms ± 3%    24.3ms ± 2%   +5.28%  (p=0.016 n=5+5)
Large/goleveldb-1000000-100-16-40/query-no-in-tree-guarantee-16    21.5µs ±11%     5.7µs ± 2%  -73.60%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-hits-16                    21.8µs ± 5%     2.7µs ±28%  -87.75%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/iteration-16                      16.6s ±11%      0.5s ± 1%  -96.81%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/update-16                         245µs ±11%     269µs ±10%     ~     (p=0.222 n=5+5)
Large/goleveldb-1000000-100-16-40/block-16                         28.8ms ± 9%    31.8ms ±19%     ~     (p=0.151 n=5+5)

name                                                             old alloc/op   new alloc/op   delta
Large/memdb-1000000-100-16-40/query-no-in-tree-guarantee-16          566B ±37%       88B ± 0%  -84.45%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/query-hits-16                          606B ± 0%       67B ±46%  -88.91%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/iteration-16                          888MB ± 0%     184MB ± 0%  -79.28%  (p=0.016 n=4+5)
Large/memdb-1000000-100-16-40/update-16                            36.7kB ± 3%    37.2kB ± 3%     ~     (p=0.222 n=5+5)
Large/memdb-1000000-100-16-40/block-16                             3.80MB ± 1%    3.86MB ± 1%   +1.62%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-no-in-tree-guarantee-16    2.72kB ±29%    0.99kB ± 0%  -63.51%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-hits-16                    2.70kB ± 1%    0.34kB ±46%  -87.34%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/iteration-16                     3.71GB ± 0%    0.30GB ± 0%  -92.00%  (p=0.016 n=4+5)
Large/goleveldb-1000000-100-16-40/update-16                        79.8kB ± 6%    84.1kB ±10%     ~     (p=0.421 n=5+5)
Large/goleveldb-1000000-100-16-40/block-16                         9.05MB ± 9%    9.82MB ±16%     ~     (p=0.095 n=5+5)

name                                                             old allocs/op  new allocs/op  delta
Large/memdb-1000000-100-16-40/query-no-in-tree-guarantee-16          10.8 ±39%       3.0 ± 0%  -72.22%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/query-hits-16                          11.0 ± 0%       1.0 ± 0%  -90.91%  (p=0.000 n=5+4)
Large/memdb-1000000-100-16-40/iteration-16                          16.0M ± 0%      3.0M ± 0%  -81.25%  (p=0.016 n=4+5)
Large/memdb-1000000-100-16-40/update-16                               466 ± 8%       459 ± 8%     ~     (p=1.000 n=5+5)
Large/memdb-1000000-100-16-40/block-16                              52.0k ± 0%     53.2k ± 0%   +2.29%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-no-in-tree-guarantee-16      37.2 ±29%      19.0 ± 0%  -48.92%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-hits-16                      36.0 ± 0%       6.0 ±50%  -83.33%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/iteration-16                      49.8M ± 0%      5.3M ± 0%  -89.34%  (p=0.016 n=4+5)
Large/goleveldb-1000000-100-16-40/update-16                           684 ± 6%       729 ±12%     ~     (p=0.310 n=5+5)
Large/goleveldb-1000000-100-16-40/block-16                          81.5k ± 7%     84.8k ±12%     ~     (p=0.421 n=5+5)

Old Benchmark
Date: 2022-01-22 12:33 AM PST
Branch: dev/iavl_data_locality with some modifications to the bench tests

go version go1.17.6 linux/amd64

Init Tree took 78.75 MB
goos: linux
goarch: amd64
pkg: github.com/cosmos/iavl/benchmarks
cpu: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz
BenchmarkMedium/memdb-100000-100-16-40/query-no-in-tree-guarantee-slow-8         	   95841	     12315 ns/op	     592 B/op	      12 allocs/op
BenchmarkMedium/memdb-100000-100-16-40/query-hits-slow-8                         	   85990	     15533 ns/op	     760 B/op	      15 allocs/op
BenchmarkMedium/memdb-100000-100-16-40/iteration-slow-8                          	       2	 838458850 ns/op	88841084 B/op	 1600092 allocs/op
--- BENCH: BenchmarkMedium/memdb-100000-100-16-40/iteration-slow-8
    bench_test.go:109: completed 100000 iterations
    bench_test.go:109: completed 100000 iterations
    bench_test.go:109: completed 100000 iterations
BenchmarkMedium/memdb-100000-100-16-40/update-8                                  	   10000	    148915 ns/op	   27312 B/op	     335 allocs/op
BenchmarkMedium/memdb-100000-100-16-40/block-8                                   	      76	  16865728 ns/op	 2910568 B/op	   35668 allocs/op
Init Tree took 46.71 MB
BenchmarkMedium/goleveldb-100000-100-16-40/query-no-in-tree-guarantee-slow-8     	   55309	     22354 ns/op	    1550 B/op	      30 allocs/op
BenchmarkMedium/goleveldb-100000-100-16-40/query-hits-slow-8                     	   43566	     27137 ns/op	    2093 B/op	      39 allocs/op
BenchmarkMedium/goleveldb-100000-100-16-40/iteration-slow-8                      	       1	2285116100 ns/op	225813440 B/op	 3857215 allocs/op
--- BENCH: BenchmarkMedium/goleveldb-100000-100-16-40/iteration-slow-8
    bench_test.go:109: completed 100000 iterations
BenchmarkMedium/goleveldb-100000-100-16-40/update-8                              	    6194	    307266 ns/op	   40138 B/op	     406 allocs/op
BenchmarkMedium/goleveldb-100000-100-16-40/block-8                               	      28	  40663600 ns/op	 5150422 B/op	   53771 allocs/op
PASS
ok  	github.com/cosmos/iavl/benchmarks	25.797s

Latest Benchmark
Date: 2022-01-22 10:15 AM PST
Branch: roman/fast-node-get-set

go version go1.17.6 linux/amd64

Init Tree took 114.29 MB
goos: linux
goarch: amd64
pkg: github.com/cosmos/iavl/benchmarks
cpu: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz
BenchmarkMedium/memdb-100000-100-16-40/query-no-in-tree-guarantee-fast-8         	  672999	      1887 ns/op	     112 B/op	       4 allocs/op
BenchmarkMedium/memdb-100000-100-16-40/query-no-in-tree-guarantee-slow-8         	   95888	     11884 ns/op	     440 B/op	       8 allocs/op
BenchmarkMedium/memdb-100000-100-16-40/query-hits-fast-8                         	  891831	      1208 ns/op	      16 B/op	       0 allocs/op
BenchmarkMedium/memdb-100000-100-16-40/query-hits-slow-8                         	   79842	     15644 ns/op	     607 B/op	      11 allocs/op
BenchmarkMedium/memdb-100000-100-16-40/iteration-fast-8                          	      20	  63956090 ns/op	18400254 B/op	  300000 allocs/op
--- BENCH: BenchmarkMedium/memdb-100000-100-16-40/iteration-fast-8
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
	... [output truncated]
BenchmarkMedium/memdb-100000-100-16-40/iteration-slow-8                          	       2	 947611750 ns/op	88841044 B/op	 1600092 allocs/op
--- BENCH: BenchmarkMedium/memdb-100000-100-16-40/iteration-slow-8
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
BenchmarkMedium/memdb-100000-100-16-40/update-8                                  	    9198	    174306 ns/op	   27524 B/op	     342 allocs/op
BenchmarkMedium/memdb-100000-100-16-40/block-8                                   	      58	  19855266 ns/op	 2948779 B/op	   36495 allocs/op
Init Tree took 66.82 MB
BenchmarkMedium/goleveldb-100000-100-16-40/query-no-in-tree-guarantee-fast-8     	  228343	      4938 ns/op	     814 B/op	      16 allocs/op
BenchmarkMedium/goleveldb-100000-100-16-40/query-no-in-tree-guarantee-slow-8     	   59304	     18046 ns/op	    1420 B/op	      24 allocs/op
BenchmarkMedium/goleveldb-100000-100-16-40/query-hits-fast-8                     	  611349	      1684 ns/op	      93 B/op	       1 allocs/op
BenchmarkMedium/goleveldb-100000-100-16-40/query-hits-slow-8                     	   50778	     23126 ns/op	    2005 B/op	      34 allocs/op
BenchmarkMedium/goleveldb-100000-100-16-40/iteration-fast-8                      	      12	  94702442 ns/op	29327220 B/op	  522988 allocs/op
--- BENCH: BenchmarkMedium/goleveldb-100000-100-16-40/iteration-fast-8
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
    bench_test.go:117: completed 100000 iterations
	... [output truncated]
BenchmarkMedium/goleveldb-100000-100-16-40/iteration-slow-8                      	       1	1716585400 ns/op	235504072 B/op	 3998006 allocs/op
--- BENCH: BenchmarkMedium/goleveldb-100000-100-16-40/iteration-slow-8
    bench_test.go:117: completed 100000 iterations
BenchmarkMedium/goleveldb-100000-100-16-40/update-8                              	    8994	    257683 ns/op	   44702 B/op	     447 allocs/op
BenchmarkMedium/goleveldb-100000-100-16-40/block-8                               	      31	  44907345 ns/op	 6973362 B/op	   72924 allocs/op
PASS
ok  	github.com/cosmos/iavl/benchmarks	43.513s

Benchmarks Interpretation
Highlighting the difference in performance from the latest benchmarks:

Old branch is dev/iavl_data_locality
New branch is roman/fast-node-get-set

Initial size: 100,000 key-val pairs
Block size: 100 keys
Key length: 16 bytes
Value length: 40 bytes

Query with no guarantee of the key being in the tree:

  • Old: 22354 ns/op
  • New on regular logic: 18046 ns/op
  • New on fast logic: 4938 ns/op
  • New fast logic shows a 77% decrease in time relative to the old branch

Query with the key guaranteed to be in the latest tree:

  • Old: 27137 ns/op
  • New on regular logic: 23126 ns/op
  • New on fast logic: 1684 ns/op
  • New fast logic shows a 93% decrease in time relative to the old branch

Iteration:

  • Old: 2285116100 ns/op
  • New on old logic: 1716585400 ns/op
  • New on fast logic: 94702442 ns/op
  • New fast logic shows a 96% decrease in time relative to the old branch

Update:
run Set, if this is a try that is divisible by blockSize, attempt to SaveVersion and if the latest saved version number history exceeds 20, delete the oldest version

  • Old: 307266 ns/op
  • New: 257683 ns/op
  • New logic shows a 16% decrease in time relative to the old branch

Block:
for block size, run Get and Set. At the end of the block, SaveVersion and if the latest saved version number history exceeds 20, delete the oldest version

  • Old: 40663600 ns/op
  • New: 44907345 ns/op
  • New logic shows a 9% increase in time relative to the old branch

ValarDragon and others added 30 commits February 12, 2022 17:43
… is deleted, fix all tests but random and with index
…not being cleared when latest version is saved
@p0mvn p0mvn force-pushed the roman/iavl_data_locality branch from 2437420 to ff9f32d Compare March 28, 2022 20:15
@p0mvn
Copy link
Member Author

p0mvn commented Mar 29, 2022

Updates:

  • Added several data race fixes that have been identified by running nodes in Osmosis.
  • Also, hardcoded fast node cache size to 100000. We identified that using the same value for regular nodes cache was causing a continuous RAM growth so we capped it at a lower value. I'm currently working on a refactor to provide this value as a config. I will open another PR with the change
  • This PR is updated with the latest changes and caught up with the master branch.
  • My local tests and linter are passing. Could someone run the CI please? I don't have the permissions.
  • gobencher is failing for reasons outside of my control: https://github.com/cosmos/iavl/pull/468/checks?check_run_id=5726930260

@tac0turtle
Copy link
Member

thanks for the update, sorry for the delay I was off last week. We will get to this asap

@robert-zaremba
Copy link
Collaborator

Could you run the benchmarks with https://pkg.go.dev/golang.org/x/perf/cmd/benchstat ?

@p0mvn
Copy link
Member Author

p0mvn commented Apr 8, 2022

Better benchstat summary:

~/projects/iavl (master)$ benchstat bench_old_large.log bench_fast_large.log
name                                                             old time/op    new time/op    delta
Large/memdb-1000000-100-16-40/query-no-in-tree-guarantee-16        11.6µs ±18%     2.1µs ± 6%  -81.86%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/query-hits-16                        13.0µs ± 3%     1.5µs ±21%  -88.06%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/iteration-16                          7.05s ± 9%     0.52s ± 2%  -92.67%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/update-16                             220µs ±10%     218µs ± 9%     ~     (p=0.841 n=5+5)
Large/memdb-1000000-100-16-40/block-16                             23.0ms ± 3%    24.3ms ± 2%   +5.28%  (p=0.016 n=5+5)
Large/goleveldb-1000000-100-16-40/query-no-in-tree-guarantee-16    21.5µs ±11%     5.7µs ± 2%  -73.60%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-hits-16                    21.8µs ± 5%     2.7µs ±28%  -87.75%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/iteration-16                      16.6s ±11%      0.5s ± 1%  -96.81%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/update-16                         245µs ±11%     269µs ±10%     ~     (p=0.222 n=5+5)
Large/goleveldb-1000000-100-16-40/block-16                         28.8ms ± 9%    31.8ms ±19%     ~     (p=0.151 n=5+5)

name                                                             old alloc/op   new alloc/op   delta
Large/memdb-1000000-100-16-40/query-no-in-tree-guarantee-16          566B ±37%       88B ± 0%  -84.45%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/query-hits-16                          606B ± 0%       67B ±46%  -88.91%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/iteration-16                          888MB ± 0%     184MB ± 0%  -79.28%  (p=0.016 n=4+5)
Large/memdb-1000000-100-16-40/update-16                            36.7kB ± 3%    37.2kB ± 3%     ~     (p=0.222 n=5+5)
Large/memdb-1000000-100-16-40/block-16                             3.80MB ± 1%    3.86MB ± 1%   +1.62%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-no-in-tree-guarantee-16    2.72kB ±29%    0.99kB ± 0%  -63.51%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-hits-16                    2.70kB ± 1%    0.34kB ±46%  -87.34%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/iteration-16                     3.71GB ± 0%    0.30GB ± 0%  -92.00%  (p=0.016 n=4+5)
Large/goleveldb-1000000-100-16-40/update-16                        79.8kB ± 6%    84.1kB ±10%     ~     (p=0.421 n=5+5)
Large/goleveldb-1000000-100-16-40/block-16                         9.05MB ± 9%    9.82MB ±16%     ~     (p=0.095 n=5+5)

name                                                             old allocs/op  new allocs/op  delta
Large/memdb-1000000-100-16-40/query-no-in-tree-guarantee-16          10.8 ±39%       3.0 ± 0%  -72.22%  (p=0.008 n=5+5)
Large/memdb-1000000-100-16-40/query-hits-16                          11.0 ± 0%       1.0 ± 0%  -90.91%  (p=0.000 n=5+4)
Large/memdb-1000000-100-16-40/iteration-16                          16.0M ± 0%      3.0M ± 0%  -81.25%  (p=0.016 n=4+5)
Large/memdb-1000000-100-16-40/update-16                               466 ± 8%       459 ± 8%     ~     (p=1.000 n=5+5)
Large/memdb-1000000-100-16-40/block-16                              52.0k ± 0%     53.2k ± 0%   +2.29%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-no-in-tree-guarantee-16      37.2 ±29%      19.0 ± 0%  -48.92%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/query-hits-16                      36.0 ± 0%       6.0 ±50%  -83.33%  (p=0.008 n=5+5)
Large/goleveldb-1000000-100-16-40/iteration-16                      49.8M ± 0%      5.3M ± 0%  -89.34%  (p=0.016 n=4+5)
Large/goleveldb-1000000-100-16-40/update-16                           684 ± 6%       729 ±12%     ~     (p=0.310 n=5+5)
Large/goleveldb-1000000-100-16-40/block-16                          81.5k ± 7%     84.8k ±12%     ~     (p=0.421 n=5+5)

Since the API has changes and bench tests were rewritten between master and the current branch, I had to create a custom version of bench_test.go that works for both branches.

@robert-zaremba I will do a new one in the next few days, sorry for the delay. However, here's the latest one.

@tac0turtle tac0turtle merged commit 0dcb21b into cosmos:master Apr 9, 2022
@Ywmet Ywmet mentioned this pull request Jul 8, 2022
@lyh169
Copy link

lyh169 commented Jul 25, 2022

screenshot-20220725-180014

hi, why modify mtx from sync.RWMutex to sync.Mutex type . I think the sync.RWMutex should also work

@marbar3778 thanks

@robert-zaremba
Copy link
Collaborator

@lyh169 there was a benchmark suggesting that. (normal Mutex was faster)

orphans map[string]int64 // Nodes removed by changes to working tree.
versions map[int64]bool // The previous, saved versions of the tree.
allRootLoaded bool // Whether all roots are loaded or not(by LazyLoadVersion)
unsavedFastNodeAdditions map[string]*FastNode // FastNodes that have not yet been saved to disk
Copy link
Contributor

@giskook giskook Aug 31, 2022

Choose a reason for hiding this comment

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

Why the unsavedFastNodeAdditions and unsavedFastNodeRemovals do not need mtx to protect? the map would not be concurrent access? @p0mvn

versions map[int64]bool // The previous, saved versions of the tree.
allRootLoaded bool // Whether all roots are loaded or not(by LazyLoadVersion)
unsavedFastNodeAdditions map[string]*FastNode // FastNodes that have not yet been saved to disk
unsavedFastNodeRemovals map[string]interface{} // FastNodes that have not yet been removed from disk
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, I review the code that the unsavedFastNodeRemovals' value is only bool. so why not use map[string]struct{}? @p0mvn

Copy link
Collaborator

Choose a reason for hiding this comment

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

that is a good optimization


## 0.17.3 (December 1, 2021)

### Improvements
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove the 3 lines above. We are targetting master, so #468 should go under Unreleased

{"badgerdb", 1000, 100, 4, 10},
// {"cleveldb", 1000, 100, 4, 10},
// {"boltdb", 1000, 100, 4, 10},
// {"rocksdb", 1000, 100, 4, 10},
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you enable rocksdb benchmarks as well?

versions = 32 // number of versions to generate
versionOps = 4096 // number of operations (create/update/delete) per version
versions = 8 // number of versions to generate
versionOps = 1024 // number of operations (create/update/delete) per version
Copy link
Collaborator

Choose a reason for hiding this comment

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

why changing this?

Comment on lines +15 to +27
start, end []byte

valid bool

ascending bool

err error

ndb *nodeDB

nextFastNode *FastNode

fastIterator dbm.Iterator
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
start, end []byte
valid bool
ascending bool
err error
ndb *nodeDB
nextFastNode *FastNode
fastIterator dbm.Iterator
start, end []byte
valid bool
ascending bool
err error
ndb *nodeDB
nextFastNode *FastNode
fastIterator dbm.Iterator

start, end := iter.fastIterator.Domain()

if start != nil {
start = start[1:]
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we remove the first byte? Could you add a doc comment?

iter.valid = iter.valid && iter.fastIterator.Valid()
if iter.valid {
iter.nextFastNode, iter.err = DeserializeFastNode(iter.fastIterator.Key()[1:], iter.fastIterator.Value())
iter.valid = iter.err == nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't we check iter.fastIterator.Valid() here as well?

@robert-zaremba
Copy link
Collaborator

wow, I've just realized that I had not submitted comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants