Skip to content

Commit

Permalink
store/cachekv: use typed types/kv.List instead of container/list.List (
Browse files Browse the repository at this point in the history
…#8811)

Reduces CPU burn by using a typed List to avoid the expensive type
assertions from using an interface. This is the only option for now
until Go makes generics generally available.

The change brings time spent on the time assertion cummulatively to:
    580ms down from 6.88s

Explanation:
The type assertions were showing up heavily in profiles:
* Before this commit
```shell
Total: 42.18s
ROUTINE ======================== github.com/cosmos/cosmos-sdk/store/cachekv.newMemIterator
in /Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/store/cachekv/memiterator.go
    14.01s     18.87s (flat, cum) 44.74% of Total
         .          .     17:	items      []*kv.Pair
         .          .     18:	ascending  bool
         .          .     19:}
         .          .     20:
         .          .     21:func newMemIterator(start, end []byte, items *list.List, ascending bool) *memIterator {
         .      620ms     22:	itemsInDomain := make([]*kv.Pair, 0, items.Len())
         .          .     23:
         .          .     24:	var entered bool
         .          .     25:
     510ms      870ms     26:	for e := items.Front(); e != nil; e = e.Next() {
     6.85s      6.88s     27:		item := e.Value.(*kv.Pair)
     5.71s      8.19s     28:		if !dbm.IsKeyInDomain(item.Key, start, end) {
     120ms      120ms     29:			if entered {
         .          .     30:				break
         .          .     31:			}
         .          .     32:
         .          .     33:			continue
         .          .     34:		}
         .          .     35:
     820ms      980ms     36:		itemsInDomain = append(itemsInDomain, item)
         .          .     37:		entered = true
         .          .     38:	}
         .          .     39:
         .      1.21s     40:	return &memIterator{
         .          .     41:		start:     start,
         .          .     42:		end:       end,
         .          .     43:		items:     itemsInDomain,
         .          .     44:		ascending: ascending,
         .          .     45:	}
```

and given that the list only uses that type, it is only right to lift the
code from container/list.List, and only modify Element.Value's type.

For emphasis, the code is basically just a retrofit of
container/list/list.go but with a typing, and we'll keep it as is
until perhaps Go1.17 or Go1.18 or when everyone uses Go1.17+ after
generics have landed.

* After this commit
```shell
Total: 45.25s
ROUTINE ======================== github.com/cosmos/cosmos-sdk/store/cachekv.newMemIterator
in /Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/store/cachekv/memiterator.go
     4.84s      6.77s (flat, cum) 14.96% of Total
         .          .     16:	items      []*kv.Pair
         .          .     17:	ascending  bool
         .          .     18:}
         .          .     19:
         .          .     20:func newMemIterator(start, end []byte, items *kv.List, ascending bool) *memIterator {
         .      330ms     21:	itemsInDomain := make([]*kv.Pair, 0, items.Len())
         .          .     22:
         .          .     23:	var entered bool
         .          .     24:
      60ms      160ms     25:	for e := items.Front(); e != nil; e = e.Next() {
     580ms      580ms     26:		item := e.Value
     3.68s      4.78s     27:		if !dbm.IsKeyInDomain(item.Key, start, end) {
      80ms       80ms     28:			if entered {
         .          .     29:				break
         .          .     30:			}
         .          .     31:
         .          .     32:			continue
         .          .     33:		}
         .          .     34:
     440ms      580ms     35:		itemsInDomain = append(itemsInDomain, item)
         .          .     36:		entered = true
         .          .     37:	}
         .          .     38:
         .      260ms     39:	return &memIterator{
         .          .     40:		start:     start,
         .          .     41:		end:       end,
         .          .     42:		items:     itemsInDomain,
         .          .     43:		ascending: ascending,
         .          .     44:	}
```

Fixes #8810
  • Loading branch information
odeke-em authored Mar 8, 2021
1 parent be23295 commit c2d5b24
Show file tree
Hide file tree
Showing 4 changed files with 245 additions and 9 deletions.
5 changes: 2 additions & 3 deletions store/cachekv/memiterator.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cachekv

import (
"container/list"
"errors"

dbm "github.com/tendermint/tm-db"
Expand All @@ -18,13 +17,13 @@ type memIterator struct {
ascending bool
}

func newMemIterator(start, end []byte, items *list.List, ascending bool) *memIterator {
func newMemIterator(start, end []byte, items *kv.List, ascending bool) *memIterator {
itemsInDomain := make([]*kv.Pair, 0, items.Len())

var entered bool

for e := items.Front(); e != nil; e = e.Next() {
item := e.Value.(*kv.Pair)
item := e.Value
if !dbm.IsKeyInDomain(item.Key, start, end) {
if entered {
break
Expand Down
9 changes: 4 additions & 5 deletions store/cachekv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cachekv

import (
"bytes"
"container/list"
"io"
"sort"
"sync"
Expand Down Expand Up @@ -30,7 +29,7 @@ type Store struct {
mtx sync.Mutex
cache map[string]*cValue
unsortedCache map[string]struct{}
sortedCache *list.List // always ascending sorted
sortedCache *kv.List // always ascending sorted
parent types.KVStore
}

Expand All @@ -41,7 +40,7 @@ func NewStore(parent types.KVStore) *Store {
return &Store{
cache: make(map[string]*cValue),
unsortedCache: make(map[string]struct{}),
sortedCache: list.New(),
sortedCache: kv.NewList(),
parent: parent,
}
}
Expand Down Expand Up @@ -134,7 +133,7 @@ func (store *Store) Write() {
// Clear the cache
store.cache = make(map[string]*cValue)
store.unsortedCache = make(map[string]struct{})
store.sortedCache = list.New()
store.sortedCache = kv.NewList()
}

// CacheWrap implements CacheWrapper.
Expand Down Expand Up @@ -206,7 +205,7 @@ func (store *Store) dirtyItems(start, end []byte) {

for e := store.sortedCache.Front(); e != nil && len(unsorted) != 0; {
uitem := unsorted[0]
sitem := e.Value.(*kv.Pair)
sitem := e.Value
comp := bytes.Compare(uitem.Key, sitem.Key)

switch comp {
Expand Down
4 changes: 3 additions & 1 deletion types/kv/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,6 @@ func (kvs Pairs) Less(i, j int) bool {
}

func (kvs Pairs) Swap(i, j int) { kvs.Pairs[i], kvs.Pairs[j] = kvs.Pairs[j], kvs.Pairs[i] }
func (kvs Pairs) Sort() { sort.Sort(kvs) }

// Sort invokes sort.Sort on kvs.
func (kvs Pairs) Sort() { sort.Sort(kvs) }
236 changes: 236 additions & 0 deletions types/kv/list.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,236 @@
package kv

// This code was copied from golang.org/pkg/container/list, but specially adapted
// for use with kv.Pair to avoid the type assertion CPU expense of using Value with
// an interface, per https://github.com/cosmos/cosmos-sdk/issues/8810
//
// Copyright 2009 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Element is an element of a linked list.
type Element struct {
// Next and previous pointers in the doubly-linked list of elements.
// To simplify the implementation, internally a list l is implemented
// as a ring, such that &l.root is both the next element of the last
// list element (l.Back()) and the previous element of the first list
// element (l.Front()).
next, prev *Element

// The list to which this element belongs.
list *List

// The value stored with this element.
Value *Pair
}

// Next returns the next list element or nil.
func (e *Element) Next() *Element {
if p := e.next; e.list != nil && p != &e.list.root {
return p
}
return nil
}

// Prev returns the previous list element or nil.
func (e *Element) Prev() *Element {
if p := e.prev; e.list != nil && p != &e.list.root {
return p
}
return nil
}

// List represents a doubly linked list.
// The zero value for List is an empty list ready to use.
type List struct {
root Element // sentinel list element, only &root, root.prev, and root.next are used
len int // current list length excluding (this) sentinel element
}

// Init initializes or clears list l.
func (l *List) Init() *List {
l.root.next = &l.root
l.root.prev = &l.root
l.len = 0
return l
}

// NewList returns an initialized list.
func NewList() *List { return new(List).Init() }

// Len returns the number of elements of list l.
// The complexity is O(1).
func (l *List) Len() int { return l.len }

// Front returns the first element of list l or nil if the list is empty.
func (l *List) Front() *Element {
if l.len == 0 {
return nil
}
return l.root.next
}

// Back returns the last element of list l or nil if the list is empty.
func (l *List) Back() *Element {
if l.len == 0 {
return nil
}
return l.root.prev
}

// lazyInit lazily initializes a zero List value.
func (l *List) lazyInit() {
if l.root.next == nil {
l.Init()
}
}

// insert inserts e after at, increments l.len, and returns e.
func (l *List) insert(e, at *Element) *Element {
e.prev = at
e.next = at.next
e.prev.next = e
e.next.prev = e
e.list = l
l.len++
return e
}

// insertValue is a convenience wrapper for insert(&Element{Value: v}, at).
func (l *List) insertValue(v *Pair, at *Element) *Element {
return l.insert(&Element{Value: v}, at)
}

// remove removes e from its list, decrements l.len, and returns e.
func (l *List) remove(e *Element) *Element {
e.prev.next = e.next
e.next.prev = e.prev
e.next = nil // avoid memory leaks
e.prev = nil // avoid memory leaks
e.list = nil
l.len--
return e
}

// move moves e to next to at and returns e.
// nolint: unparam
func (l *List) move(e, at *Element) *Element {
if e == at {
return e
}
e.prev.next = e.next
e.next.prev = e.prev

e.prev = at
e.next = at.next
e.prev.next = e
e.next.prev = e

return e
}

// Remove removes e from l if e is an element of list l.
// It returns the element value e.Value.
// The element must not be nil.
func (l *List) Remove(e *Element) *Pair {
if e.list == l {
// if e.list == l, l must have been initialized when e was inserted
// in l or l == nil (e is a zero Element) and l.remove will crash
l.remove(e)
}
return e.Value
}

// PushFront inserts a new element e with value v at the front of list l and returns e.
func (l *List) PushFront(v *Pair) *Element {
l.lazyInit()
return l.insertValue(v, &l.root)
}

// PushBack inserts a new element e with value v at the back of list l and returns e.
func (l *List) PushBack(v *Pair) *Element {
l.lazyInit()
return l.insertValue(v, l.root.prev)
}

// InsertBefore inserts a new element e with value v immediately before mark and returns e.
// If mark is not an element of l, the list is not modified.
// The mark must not be nil.
func (l *List) InsertBefore(v *Pair, mark *Element) *Element {
if mark.list != l {
return nil
}
// see comment in List.Remove about initialization of l
return l.insertValue(v, mark.prev)
}

// InsertAfter inserts a new element e with value v immediately after mark and returns e.
// If mark is not an element of l, the list is not modified.
// The mark must not be nil.
func (l *List) InsertAfter(v *Pair, mark *Element) *Element {
if mark.list != l {
return nil
}
// see comment in List.Remove about initialization of l
return l.insertValue(v, mark)
}

// MoveToFront moves element e to the front of list l.
// If e is not an element of l, the list is not modified.
// The element must not be nil.
func (l *List) MoveToFront(e *Element) {
if e.list != l || l.root.next == e {
return
}
// see comment in List.Remove about initialization of l
l.move(e, &l.root)
}

// MoveToBack moves element e to the back of list l.
// If e is not an element of l, the list is not modified.
// The element must not be nil.
func (l *List) MoveToBack(e *Element) {
if e.list != l || l.root.prev == e {
return
}
// see comment in List.Remove about initialization of l
l.move(e, l.root.prev)
}

// MoveBefore moves element e to its new position before mark.
// If e or mark is not an element of l, or e == mark, the list is not modified.
// The element and mark must not be nil.
func (l *List) MoveBefore(e, mark *Element) {
if e.list != l || e == mark || mark.list != l {
return
}
l.move(e, mark.prev)
}

// MoveAfter moves element e to its new position after mark.
// If e or mark is not an element of l, or e == mark, the list is not modified.
// The element and mark must not be nil.
func (l *List) MoveAfter(e, mark *Element) {
if e.list != l || e == mark || mark.list != l {
return
}
l.move(e, mark)
}

// PushBackList inserts a copy of another list at the back of list l.
// The lists l and other may be the same. They must not be nil.
func (l *List) PushBackList(other *List) {
l.lazyInit()
for i, e := other.Len(), other.Front(); i > 0; i, e = i-1, e.Next() {
l.insertValue(e.Value, l.root.prev)
}
}

// PushFrontList inserts a copy of another list at the front of list l.
// The lists l and other may be the same. They must not be nil.
func (l *List) PushFrontList(other *List) {
l.lazyInit()
for i, e := other.Len(), other.Back(); i > 0; i, e = i-1, e.Prev() {
l.insertValue(e.Value, &l.root)
}
}

0 comments on commit c2d5b24

Please sign in to comment.