Skip to content

perf: optimize iteration on nested cache context (backport #13881)#14341

Merged
tac0turtle merged 5 commits intorelease/v0.45.xfrom
mergify/bp/release/v0.45.x/pr-13881
Jan 5, 2023
Merged

perf: optimize iteration on nested cache context (backport #13881)#14341
tac0turtle merged 5 commits intorelease/v0.45.xfrom
mergify/bp/release/v0.45.x/pr-13881

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify bot commented Dec 16, 2022

This is an automatic backport of pull request #13881 done by Mergify.
Cherry-pick of cbee1b3 has failed:

On branch mergify/bp/release/v0.45.x/pr-13881
Your branch is up to date with 'origin/release/v0.45.x'.

You are currently cherry-picking commit cbee1b3ea.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   store/cachekv/benchmark_test.go
	new file:   store/cachekv/internal/btree.go
	new file:   store/cachekv/internal/btree_test.go
	new file:   store/cachekv/internal/memiterator.go
	renamed:    store/cachekv/mergeiterator.go -> store/cachekv/internal/mergeiterator.go
	modified:   store/cachekv/search_benchmark_test.go

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   CHANGELOG.md
	both modified:   go.mod
	both modified:   go.sum
	deleted by us:   simapp/go.mod
	deleted by us:   simapp/go.sum
	deleted by them: store/cachekv/memiterator.go
	both modified:   store/cachekv/store.go
	both modified:   store/cachekv/store_test.go
	deleted by us:   tests/go.mod
	deleted by us:   tests/go.sum

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@mergify
perf: optimize iteration on nested cache context (#13881)
d98c8ec 
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Marko <marbar3778@yahoo.com>
Closes #10310

(cherry picked from commit cbee1b3)

# Conflicts:
#	CHANGELOG.md
#	go.mod
#	go.sum
#	simapp/go.mod
#	simapp/go.sum
#	store/cachekv/memiterator.go
#	store/cachekv/store.go
#	store/cachekv/store_test.go
#	tests/go.mod
#	tests/go.sum
@mergify mergify bot added the conflicts label Dec 16, 2022
@mergify mergify bot mentioned this pull request Dec 16, 2022
Merged
19 tasks
@tac0turtle tac0turtle closed this Dec 16, 2022
@mergify mergify bot deleted the mergify/bp/release/v0.45.x/pr-13881 branch December 16, 2022 13:39
@tac0turtle tac0turtle restored the mergify/bp/release/v0.45.x/pr-13881 branch December 17, 2022 13:59
@tac0turtle tac0turtle reopened this Dec 17, 2022
@tac0turtle tac0turtle requested a review from yihuang December 17, 2022 14:05
yihuang
yihuang approved these changes Dec 31, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@yihuang yihuang left a comment

Choose a reason for hiding this comment

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

LGTM

@yihuang yihuang removed the conflicts label Dec 31, 2022
@julienrbrt
Copy link
Copy Markdown
Contributor

julienrbrt commented Jan 2, 2023

TestNilEndIterator is still failing here: https://github.com/cosmos/cosmos-sdk/actions/runs/3812410275/jobs/6485601037

@likhita-809
Copy link
Copy Markdown
Contributor

likhita-809 commented Jan 4, 2023

@yihuang can you look into the failing test TestNilEndIterator ?

@kocubinski
fix(store): handle nil end in cachekv/iterator (#12945)
2279a7c 
Closes: #12661

Adds support for nil end semantics to iterators in cachekv store, addressing [this workaround](https://github.com/osmosis-labs/osmosis/blob/4176b287d48338870bfda3029bfa20a6e45ac126/osmoutils/store_helper.go#L37-L41).

Note that this has the effect of sorting the dirty cache into the BTree cache store in the bounds `[startIndex, end-of-cache-space]`

---

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
@yihuang
Copy link
Copy Markdown
Collaborator

yihuang commented Jan 5, 2023 •
edited
Loading

@yihuang can you look into the failing test TestNilEndIterator ?

fixed, just need to backport #12945 together.

@tac0turtle tac0turtle merged commit 77e66cf into release/v0.45.x Jan 5, 2023
@tac0turtle tac0turtle deleted the mergify/bp/release/v0.45.x/pr-13881 branch January 5, 2023 10:52
@faddat faddat mentioned this pull request Mar 23, 2023
Closed
19 tasks
@faddat faddat mentioned this pull request Dec 11, 2023
Draft
19 tasks
JeancarloBarrios pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Sep 28, 2024
@mergify @tac0turtle
@kocubinski @JeancarloBarrios
perf: optimize iteration on nested cache context (backport cosmos#13881…
100f102 
…) (cosmos#14341)

Co-authored-by: yihuang <huang@crypto.com>
Co-authored-by: marbar3778 <marbar3778@yahoo.com>
Co-authored-by: Matt Kocubinski <mkocubinski@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yihuang yihuang yihuang approved these changes

@aaronc aaronc Awaiting requested review from aaronc aaronc is a code owner

@alexanderbez alexanderbez Awaiting requested review from alexanderbez

Assignees

@facundomedica facundomedica

@likhita-809 likhita-809

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@julienrbrt @likhita-809 @yihuang @facundomedica @tac0turtle @kocubinski