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

store/cachekv: use typed types/kv.List instead of container/list.List #8811

Merged
merged 2 commits into from
Mar 8, 2021

Conversation

odeke-em
Copy link
Collaborator

@odeke-em odeke-em commented Mar 6, 2021

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
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
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


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

@codecov
Copy link

codecov bot commented Mar 6, 2021

Codecov Report

Merging #8811 (fd18e9f) into master (be23295) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8811   +/-   ##
=======================================
  Coverage   59.45%   59.45%           
=======================================
  Files         563      563           
  Lines       31277    31277           
=======================================
  Hits        18597    18597           
  Misses      10518    10518           
  Partials     2162     2162           
Impacted Files Coverage Δ
store/cachekv/memiterator.go 65.78% <100.00%> (ø)
store/cachekv/store.go 88.17% <100.00%> (ø)

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

ACK - pending linter fixes

@odeke-em odeke-em force-pushed the typed-kv.List-adapted-from-container.list branch 5 times, most recently from 9acc923 to 9e2a327 Compare March 7, 2021 03:45
@odeke-em
Copy link
Collaborator Author

odeke-em commented Mar 7, 2021

The golint-ci error is just silly

  Error: (*List).move - result 0 (*github.com/cosmos/cosmos-sdk/types/kv.Element) is never used (unparam)

for code that looks like this

// move moves e to next to at and returns e.
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
}

just super pedantic and I'd consider that warning a bug on golint-ci itself, the official golint doesn't complain, plus this code is from the standard library.

@odeke-em odeke-em force-pushed the typed-kv.List-adapted-from-container.list branch from 9e2a327 to 780c30a Compare March 7, 2021 03:53
Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

utACK.

Loving seeing this work land in the sdk. Excited to see more things.

@odeke-em odeke-em force-pushed the typed-kv.List-adapted-from-container.list branch from 780c30a to 0c4e45f Compare March 7, 2021 12:29
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
@odeke-em odeke-em force-pushed the typed-kv.List-adapted-from-container.list branch from 0c4e45f to e3ea4cd Compare March 7, 2021 12:30
@odeke-em
Copy link
Collaborator Author

odeke-em commented Mar 7, 2021

Thank you for the reviews @alessio and @marbar3778. Indeed, more coming :-)

Copy link
Contributor

@fdymylja fdymylja left a comment

Choose a reason for hiding this comment

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

utACK, amazing work! it's always a pleasure to follow your detailed profiling :)

@odeke-em
Copy link
Collaborator Author

odeke-em commented Mar 8, 2021

Thank you for the review @fdymylja, and for the kind words.

@odeke-em odeke-em merged commit c2d5b24 into master Mar 8, 2021
@odeke-em odeke-em deleted the typed-kv.List-adapted-from-container.list branch March 8, 2021 17:16
@odeke-em
Copy link
Collaborator Author

odeke-em commented Mar 8, 2021

Could we perhaps backport this change?

@alessio
Copy link
Contributor

alessio commented Mar 8, 2021

I'm afraid it's too late for 0.42.0, it will surely make it into v0.42.1 too!

mergify bot pushed a commit that referenced this pull request Mar 8, 2021
…#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

(cherry picked from commit c2d5b24)
charleenfei pushed a commit to charleenfei/cosmos-sdk that referenced this pull request Mar 10, 2021
…cosmos#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 cosmos#8810
alessio pushed a commit that referenced this pull request Mar 10, 2021
…#8811) (#8817)

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

(cherry picked from commit c2d5b24)

Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
@robert-zaremba
Copy link
Collaborator

Good job. Wow, that's a big surprise to me. So the type assertion / cast was a bottleneck here?
It's kind of sad that we need to copy the whole file just to use a concrete type. I always thought that interfaces check is just a dereference + an integer comparison in the runtime.

mergify bot added a commit that referenced this pull request Mar 10, 2021
* fixed broken links, typos

* Update docs/ibc/overview.md

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

* Update docs/intro/sdk-app-architecture.md

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* Update docs/building-modules/simulator.md

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* build(deps): bump JamesIves/github-pages-deploy-action from 4.0.0 to 4.1.0 (#8792)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/JamesIves/github-pages-deploy-action) from 4.0.0 to 4.1.0.
- [Release notes](https://github.com/JamesIves/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@4.0.0...3dbacc7)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix multisig account pubkeys migration (#8794)

closes: #8776

* Update mergify (#8784)

* Update mergify

Prep for the v0.42 release series.

* retire v0.41, the hub can upgrade to v0.42 smoothly

* perf change (#8796)

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Supply by denom Migrations (#8780)

* Add back supply proto

* Add migration for supply

* Fix lint

* Update x/bank/spec/01_state.md

* Fix test

* Proto gen

* Update x/bank/spec/01_state.md

* Make proto gen

Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>

* fix make protoc error (#8799)

* reduce gas costs by 10x for transient store operations (#8790)

* reduce gas costs by 10x for transient store operations

* fix TestTransientGasConfig for ReadCostFlat

* added changelog entry

* fix changelog

* fix changelog

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* x/gov: fix NormalizeProposalType() return values (#8808)

Closes: #8806

* 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

* Move all migration scripts to v043 (#8814)

* Move all migration scripts to v043

* Fix permaling

* Fix test

* Fix test again

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* permalinks

Co-authored-by: chrly <chrly@chrlys-MacBook-Pro.local>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>
Co-authored-by: Marko <markobaricevic3778@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Akhil Kumar P <36399231+akhilkumarpilli@users.noreply.github.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Albert Chon <albert@injectiveprotocol.com>
Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
mergify bot pushed a commit that referenced this pull request Mar 10, 2021
* fixed broken links, typos

* Update docs/ibc/overview.md

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

* Update docs/intro/sdk-app-architecture.md

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* Update docs/building-modules/simulator.md

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* build(deps): bump JamesIves/github-pages-deploy-action from 4.0.0 to 4.1.0 (#8792)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/JamesIves/github-pages-deploy-action) from 4.0.0 to 4.1.0.
- [Release notes](https://github.com/JamesIves/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@4.0.0...3dbacc7)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix multisig account pubkeys migration (#8794)

closes: #8776

* Update mergify (#8784)

* Update mergify

Prep for the v0.42 release series.

* retire v0.41, the hub can upgrade to v0.42 smoothly

* perf change (#8796)

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Supply by denom Migrations (#8780)

* Add back supply proto

* Add migration for supply

* Fix lint

* Update x/bank/spec/01_state.md

* Fix test

* Proto gen

* Update x/bank/spec/01_state.md

* Make proto gen

Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>

* fix make protoc error (#8799)

* reduce gas costs by 10x for transient store operations (#8790)

* reduce gas costs by 10x for transient store operations

* fix TestTransientGasConfig for ReadCostFlat

* added changelog entry

* fix changelog

* fix changelog

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* x/gov: fix NormalizeProposalType() return values (#8808)

Closes: #8806

* 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

* Move all migration scripts to v043 (#8814)

* Move all migration scripts to v043

* Fix permaling

* Fix test

* Fix test again

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* permalinks

Co-authored-by: chrly <chrly@chrlys-MacBook-Pro.local>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>
Co-authored-by: Marko <markobaricevic3778@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Akhil Kumar P <36399231+akhilkumarpilli@users.noreply.github.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Albert Chon <albert@injectiveprotocol.com>
Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
(cherry picked from commit 55fc465)
alessio pushed a commit that referenced this pull request Mar 10, 2021
* fixed broken links, typos

* Update docs/ibc/overview.md

Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>

* Update docs/intro/sdk-app-architecture.md

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* Update docs/building-modules/simulator.md

Co-authored-by: Marko <markobaricevic3778@gmail.com>

* build(deps): bump JamesIves/github-pages-deploy-action from 4.0.0 to 4.1.0 (#8792)

Bumps [JamesIves/github-pages-deploy-action](https://github.com/JamesIves/github-pages-deploy-action) from 4.0.0 to 4.1.0.
- [Release notes](https://github.com/JamesIves/github-pages-deploy-action/releases)
- [Commits](JamesIves/github-pages-deploy-action@4.0.0...3dbacc7)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix multisig account pubkeys migration (#8794)

closes: #8776

* Update mergify (#8784)

* Update mergify

Prep for the v0.42 release series.

* retire v0.41, the hub can upgrade to v0.42 smoothly

* perf change (#8796)

Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* Supply by denom Migrations (#8780)

* Add back supply proto

* Add migration for supply

* Fix lint

* Update x/bank/spec/01_state.md

* Fix test

* Proto gen

* Update x/bank/spec/01_state.md

* Make proto gen

Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>

* fix make protoc error (#8799)

* reduce gas costs by 10x for transient store operations (#8790)

* reduce gas costs by 10x for transient store operations

* fix TestTransientGasConfig for ReadCostFlat

* added changelog entry

* fix changelog

* fix changelog

Co-authored-by: Alessio Treglia <alessio@tendermint.com>

* x/gov: fix NormalizeProposalType() return values (#8808)

Closes: #8806

* 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

* Move all migration scripts to v043 (#8814)

* Move all migration scripts to v043

* Fix permaling

* Fix test

* Fix test again

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

* permalinks

Co-authored-by: chrly <chrly@chrlys-MacBook-Pro.local>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com>
Co-authored-by: Marko <markobaricevic3778@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Akhil Kumar P <36399231+akhilkumarpilli@users.noreply.github.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Albert Chon <albert@injectiveprotocol.com>
Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
(cherry picked from commit 55fc465)

Co-authored-by: Charly <charly@interchain.berlin>
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.

store/cachekv: newMemIterator and others could use a typed linked list because type assertions are expensive
5 participants