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

Refactor encode funcs #1399

Merged
merged 11 commits into from
Jan 9, 2024
Merged

Conversation

4l0n50
Copy link
Contributor

@4l0n50 4l0n50 commented Dec 1, 2023

This PR changes how branch and empty nodes are encoded:

  • The encoding of an empty node is stored at position @ENCODED_EMPTY_NODE_POS of the rlp segment.
  • For encoding a branch node we write the child encoded values directly to the rlp segment, instead of storing first the values on SEGMENT_TRIE_ENCODED_CHILD
    It saves ~2500 cycles with add11_yml.

@Nashtare Nashtare added the optimization Performance related changes label Dec 1, 2023
evm/src/cpu/kernel/asm/mpt/hash/hash.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/hash/hash.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/hash/hash.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/hash/hash.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/util.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/util.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/constants/mod.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/constants/mod.rs Outdated Show resolved Hide resolved
Clean code

Not important

Restore jumpdets_analysis.asm

Refactor encode_empty_node and encode_branch_node
evm/src/cpu/kernel/asm/mpt/hash/hash.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/hash/hash.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/util.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/constants/mod.rs Outdated Show resolved Hide resolved
@@ -942,6 +944,18 @@ impl<'a> Interpreter<'a> {
}
self.generation_state.registers.context = context;
}

/// Writes the encoding of 0 to position @
Copy link
Contributor

Choose a reason for hiding this comment

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

@ENCODED_EMPTY_NODE_POS?

evm/src/generation/prover_input.rs Outdated Show resolved Hide resolved
Refactor encode_empty_node and encode_branch_node

Add constant and store encoded empty node in an other position

Remove child segment

Clean code

Apply suggestions from code review

Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com>

Remive global label

Move encoded empty nodes
@4l0n50
Copy link
Contributor Author

4l0n50 commented Dec 27, 2023

Sorry, I had to force push when I rebased to main

evm/src/cpu/kernel/asm/mpt/hash/hash.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/hash/hash.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/mpt/hash/hash_trie_specific.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/tests/mpt/insert.rs Outdated Show resolved Hide resolved
evm/src/generation/mod.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/constants/mod.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/constants/mod.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/main.asm Outdated Show resolved Hide resolved
4l0n50 and others added 2 commits January 5, 2024 17:18
Co-authored-by: Linda Guiga <101227802+LindaGuiga@users.noreply.github.com>
Co-authored-by: Robin Salen <30937548+Nashtare@users.noreply.github.com>
evm/src/cpu/kernel/constants/mod.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/tests/account_code.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/tests/account_code.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/tests/account_code.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/tests/account_code.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/tests/add11.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

Two minor comments for my OCD 😃

evm/src/cpu/kernel/constants/mod.rs Outdated Show resolved Hide resolved
evm/src/cpu/kernel/interpreter.rs Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Jan 9, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@4l0n50 4l0n50 merged commit 95c83ad into 0xPolygonZero:main Jan 9, 2024
4 checks passed
@Nashtare Nashtare deleted the refactor_encode_funcs branch January 9, 2024 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization Performance related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants