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

fix!: store/cachekv: reduce growth factor for iterator ranging using binary searches #10024

Merged
merged 1 commit into from
Oct 14, 2021
Merged

fix!: store/cachekv: reduce growth factor for iterator ranging using binary searches #10024

merged 1 commit into from
Oct 14, 2021

Commits on Oct 14, 2021

  1. store/cachekv: reduce growth factor for iterator ranging using binary…

    … searches
    
    This change takes the observation that previous dbm.IsKeyInDomain
    which searches for [start, end) was performing too many byteslice
    comparisons. Instead we start off by sorting all the values in the
    store.unsortedCache, and then apply a modified binary search to
    look for values that fall within the domain [start, end)
    The procedure involves:
    * iterating over all items to build a list of all keys -- O(n)
    * invoking sort.Strings immediately, of which
    we anyways eventually invoke sort.Slice(unsorted, ...) which uses
    Quicksort -- O(nlog(n)) or O(n^2) worst case
    * invoking modified binary search which is O(log(n)) * 2 ~ O(log(n))
    to search for the [start, end) range indices
    
    for a total approximate complexity of:
    Best case:  O(n) + O(n(log(n))) + O(log(n)) ~= O(nlog(n))
    Worst case: O(n) + O(n^2) + O(log(n))       ~= O(n^2)
    
    instead of previously:
    * iterating over all the unsorted items and invoking dbm.IsKeyInDomain:
    bytes.Compare ~ O(n) + O(n*s*e) where s -- len(start), e -- len(end)
    for overall complexity of O(n*s*e)
    * invoking sort.Slice(unsorted, ...) which uses
    Quicksort -- O(nlog(n)) or O(n^2) worst case
    
    for a total approximate complexity of:
    Best case:  O(n) + O(n*s*e) + O(nlog(n)) ~= O(n*s*e) ~ O(n^2)
    Worst case: O(n) + O(n*s*e) + O(n^2)     ~= O(n*s*e) ~ O(n^2)
    
    Ordinarily we'd combine the n*s*e to be n*m, but really the comparisons
    between (start & key, end & key) are profound that it makes sense to
    keep them as factors. The overall benchmark results vindicate our choice
    of isolating the factors (n*s*e)
    
    The benchmarks show that as the number of keys to iterate grows, the
    new code grows gracefully in a somewhat linear growth, notice for
    CAcheKVStoreIterator*, when we go from:
    * 1,000 to 10,000 keys: 120us->1,600us (13X) old vs 95us->900us (9.47X) new
    * 50,000 to 100,000 keys: 19ms->100ms (5.3X) old vs 5.5ms->17ms (3X) new
    ```shell
    time/op
    GetValidator-8	              5.8ms ± 2%    4.7ms ± 1%	-17.69%	(p=0.000 n=10+10)
    OneBankSendTxPerBlock-8	      3.2ms ± 2%    2.8ms ± 1%	-10.80%	(p=0.000 n=7+10)
    OneBankMultiSendTxPerBlock-8  3.1ms ± 3%    2.9ms ± 2%	-8.36%	(p=0.000 n=10+10)
    AccountMapperSetAccount-8     8.6µs ± 1%    7.8µs ± 1%	-9.74%	(p=0.000 n=10+10)
    CacheKVStoreIterator500-8     64µs ± 6%	    51µs ± 6%	-19.22%	(p=0.000 n=10+9)
    CacheKVStoreIterator1000-8    0.12ms ± 4%   95µs ± 4%	-19.55%	(p=0.000 n=10+10)
    CacheKVStoreIterator10000-8   1.6ms ± 4%    0.90ms ± 1%	-42.11%	(p=0.000 n=10+10)
    CacheKVStoreIterator50000-8   19ms ± 5%	    5.5ms ± 1%	-71.35%	(p=0.000 n=10+10)
    CacheKVStoreIterator100000-8  0.10s ± 23%   17ms ± 7%	-83.44%	(p=0.000 n=10+10)
    CacheKVStoreGetNoKeyFound-8   1.3µs ± 6%    0.90µs ± 3%	-31.19%	(p=0.000 n=9+9)
    CacheKVStoreGetKeyFound-8     0.66µs ± 6%   0.56µs ± 2%	-14.81%	(p=0.000 n=10+9)
    
    alloc/op
    B/op
    BlockProvision-8	     0.11kB ± 0%    0.10kB ± 0%	-7.14%	(p=0.000 n=10+10)
    CacheKVStoreIterator50000-8  0.89MB ± 6%    0.53MB ± 1%	-40.85%	(p=0.000 n=10+10)
    CacheKVStoreIterator100000-8 6.3MB ± 23%    1.6MB ± 6%	-74.17%	(p=0.000 n=10+10)
    CacheKVStoreGetNoKeyFound-8  0.26kB ± 0%    0.23kB ± 1%	-11.53%	(p=0.000 n=10+8)
    
    allocs/op (count)
    AccountMapperSetAccount-8    42 ± 0%	    38 ± 0%	-9.52%	(p=0.000 n=10+10)
    BlockProvision-8	     6.0 ± 0%	    5.0 ± 0%	-16.67%	(p=0.000 n=10+10)
    CacheKVStoreIterator1000-8   14 ± 0%	    13 ± 0%	-7.14%	(p=0.002 n=8+10)
    CacheKVStoreIterator10000-8  0.15k ± 2%	    76 ± 1%	-49.00%	(p=0.000 n=7+10)
    CacheKVStoreIterator50000-8  8.9k ± 11%	    2.0k ± 2%	-77.60%	(p=0.000 n=10+10)
    CacheKVStoreIterator100000-8 0.10M ± 26%    13k ± 12%	-86.89%	(p=0.000 n=10+10)
    CacheKVStoreGetNoKeyFound-8  5.0 ± 0%	    4.0 ± 0%	-20.00%	(p=0.000 n=10+10)
    ```
    
    Note: Purposefully using a commit off master that doesn't
    include the buggy code that caused x/bank.BenchmarkOneBank* to fail
    per issue #10023
    
    Updates #9876
    odeke-em committed Oct 14, 2021
    Configuration menu
    Copy the full SHA
    3ab2006 View commit details
    Browse the repository at this point in the history