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

Optimize SSZ collection tree creation #9088

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

VolodymyrBg
Copy link

I

Optimize SSZ collection tree creation

PR Description

Optimizes the createTreeFromElements method in SszCollectionSchema by implementing a direct bottom-up tree construction approach instead of using intermediate mutable copies. This change improves performance and memory efficiency when creating SSZ collections.

Key improvements:

  • Eliminates the need for intermediate mutable copies
  • Implements efficient bottom-up binary tree construction
  • Optimizes memory usage by reusing node arrays
  • Handles odd number of nodes efficiently

Fixed Issue(s)

Addresses TODO comment in SszCollectionSchema.java about suboptimal method implementation.

Documentation

  • No documentation updates required as this is an internal performance optimization.

Changelog

  • Added changelog entry:

Implements a more efficient approach to creating SSZ collection trees by
using direct bottom-up tree construction instead of intermediate mutable
copies. This optimization improves performance and memory efficiency
when creating SSZ collections from lists of elements.

The change eliminates the need for creating and modifying mutable copies,
replacing it with a direct tree construction algorithm that builds the
binary tree structure from the leaf nodes up.
@rolfyone
Copy link
Contributor

rolfyone commented Feb 6, 2025

Hey, thanks for raising.

The spotless task is a gradle target spotlessJavaApply and will get you past that point.

I think we'd probably need to look at performance metrics (not sure if a JMH currently covers this) to demonstrate an improvement from this TODO before we'd look to merge this PR...

@VolodymyrBg
Copy link
Author

Hey, thanks for raising.

The spotless task is a gradle target spotlessJavaApply and will get you past that point.

I think we'd probably need to look at performance metrics (not sure if a JMH currently covers this) to demonstrate an improvement from this TODO before we'd look to merge this PR...

Fixed

@rolfyone
Copy link
Contributor

Spotless still failing, are you still working on this?

@VolodymyrBg
Copy link
Author

Spotless still failing, are you still working on this?

Spotless is alright now but some others are failing. Do I need to correct it too?

@rolfyone
Copy link
Contributor

Spotless is alright now but some others are failing. Do I need to correct it too?

basically now it's failing to compile, so gradle build would show you the errors.

for one reeNode.ZERO_TREE_NODE wasn't found. but it looks like 3 build errors listed during build.

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