From c2d5b24f58a6537d2a7205ca4f2960c8424c4bef Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Mon, 8 Mar 2021 09:16:23 -0800 Subject: [PATCH] store/cachekv: use typed types/kv.List instead of container/list.List (#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 --- store/cachekv/memiterator.go | 5 +- store/cachekv/store.go | 9 +- types/kv/kv.go | 4 +- types/kv/list.go | 236 +++++++++++++++++++++++++++++++++++ 4 files changed, 245 insertions(+), 9 deletions(-) create mode 100644 types/kv/list.go diff --git a/store/cachekv/memiterator.go b/store/cachekv/memiterator.go index 675494b8ffe..b197ac14166 100644 --- a/store/cachekv/memiterator.go +++ b/store/cachekv/memiterator.go @@ -1,7 +1,6 @@ package cachekv import ( - "container/list" "errors" dbm "github.com/tendermint/tm-db" @@ -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 diff --git a/store/cachekv/store.go b/store/cachekv/store.go index a953d4436b1..af5d75f9c83 100644 --- a/store/cachekv/store.go +++ b/store/cachekv/store.go @@ -2,7 +2,6 @@ package cachekv import ( "bytes" - "container/list" "io" "sort" "sync" @@ -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 } @@ -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, } } @@ -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. @@ -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 { diff --git a/types/kv/kv.go b/types/kv/kv.go index a0aab3f47fe..1f3da91cc2c 100644 --- a/types/kv/kv.go +++ b/types/kv/kv.go @@ -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) } diff --git a/types/kv/list.go b/types/kv/list.go new file mode 100644 index 00000000000..9e928c84912 --- /dev/null +++ b/types/kv/list.go @@ -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) + } +}