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

implement btree.item on item itelf, not only its pointer. #254

Merged
merged 4 commits into from
May 19, 2022

Conversation

faddat
Copy link
Contributor

@faddat faddat commented May 7, 2022

Closes: #186

@ValarDragon I think that this is correct, but I did have one question

memdb.go Outdated
@@ -32,6 +32,14 @@ func (i *item) Less(other btree.Item) bool {
return bytes.Compare(i.key, other.(*item).key) == -1
}

// LessDirect implements btree.Item directly on item instead of its pointer.
// Question: is this it, or do we need to get the pointer off of other.(*item).key?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the question

@codecov
Copy link

codecov bot commented May 7, 2022

Codecov Report

Merging #254 (29ac0c5) into master (a9e1585) will not change coverage.
The diff coverage is 90.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #254   +/-   ##
=======================================
  Coverage   68.54%   68.54%           
=======================================
  Files          27       27           
  Lines        2130     2130           
=======================================
  Hits         1460     1460           
  Misses        595      595           
  Partials       75       75           
Impacted Files Coverage Δ
memdb.go 70.27% <87.50%> (ø)
memdb_iterator.go 93.40% <100.00%> (ø)
Impacted Files Coverage Δ
memdb.go 70.27% <87.50%> (ø)
memdb_iterator.go 93.40% <100.00%> (ø)

@creachadair
Copy link
Contributor

If the goal is to implement the interface on both item and *item, then we should move the methods to item alone (the non-pointer type). The method set of a pointer type includes the methods of the base type, so we don't need to double up.

It should satisfy the core request of #186 to update all the existing methods without adding any new ones. However, for it to be useful we'd probably also have to update the code that generates items to not allocate them.

@faddat
Copy link
Contributor Author

faddat commented May 19, 2022

Thanks @catShaark !

@creachadair creachadair merged commit e24b965 into tendermint:master May 19, 2022
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.

memdb: Implement btree.Item on item directly, in addition to *item
3 participants