Skip to content

simplify tree diff algorithm#12

Merged
yihuang merged 6 commits intocrypto-com:mainfrom
yihuang:refactor-diff
Dec 13, 2022
Merged

simplify tree diff algorithm#12
yihuang merged 6 commits intocrypto-com:mainfrom
yihuang:refactor-diff

Conversation

@yihuang
Copy link
Copy Markdown

@yihuang yihuang commented Dec 12, 2022

The algorithm is adapted from cosmos/iavl#646, credits: @cool-developer
The new one visit the same amount of nodes as the previous one, but is simpler.
It could be further optimized after new node key format, because we can traverse nodes with whatever order, so we can just traverse them ordered by node key with the version as prefix.

@yihuang yihuang changed the title simplify diff algorithm simplify tree diff algorithm Dec 12, 2022
@yihuang yihuang requested a review from mmsqe December 12, 2022 07:50
iavl/iavl.py Outdated
get_node: GetNode,
v: int,
predecessor: int,
successor: int,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

any usage of successor?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

good catch, cleaned up.

iavl/iavl.py Outdated
predecessor: int,
root: bytes,
successor_root: bytes,
) -> int:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should return node right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

should return nothing, fixed.

iavl/utils.py Outdated
@@ -330,26 +335,26 @@ def visit_iavl_nodes(
stack: List[bytes] = [hash]
while stack:
hash = stack.pop()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should we rename like hash_or_node?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done


node = get_node(hash)

if not preorder:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

not sure if we need postorder now, but need loop all anyway right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

postorder is only used in import/export, which we haven't implemented in python version.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

what loop all?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

unless prune_check is different

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, without prune_check it'll traverse all.

yihuang and others added 2 commits December 13, 2022 09:57
Co-authored-by: mmsqe <tqd0800210105@gmail.com>
Signed-off-by: yihuang <huang@crypto.com>
@yihuang yihuang merged commit 49a8a8b into crypto-com:main Dec 13, 2022
@yihuang yihuang deleted the refactor-diff branch December 13, 2022 03:23
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.

2 participants