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

Integrate hashtree for better hashing performance #5188

Merged
merged 12 commits into from May 2, 2024
Merged

Conversation

arnetheduck
Copy link
Member

@arnetheduck arnetheduck commented Jul 13, 2023

status-im/nim-ssz-serialization#35 brings in https://github.com/prysmaticlabs/hashtree as a supported backend for SHA256, giving a nice little performance boost to all hash_tree_root calls on supported platforms / compilers.

Expected gains are on the order of 30% which in the case of a replayed state nets us 0.2-0.3s improvement.

More about this design here: https://hackmd.io/@potuz/BJyrx9DOF - kudos to @potuz for this excellent library!

@github-actions
Copy link

github-actions bot commented Jul 13, 2023

Unit Test Results

         9 files  ±0    1 115 suites  ±0   23m 52s ⏱️ - 1m 8s
  4 245 tests ±0    3 898 ✔️ ±0  347 💤 ±0  0 ±0 
16 929 runs  ±0  16 531 ✔️ ±0  398 💤 ±0  0 ±0 

Results for commit 269e13b. ± Comparison against base commit 109007d.

♻️ This comment has been updated with latest results.

@tersec
Copy link
Contributor

tersec commented Jul 13, 2023

fatal: remote error: upload-pack: not our ref dc1992a73be5643cf4b06bd29a22478f8d32513b
Errors during submodule fetch:
	vendor/nim-ssz-serialization

probably that's not available in the git repo, but this error triggers regardless on git pull in such cases, even when not having checked out this branch specifically

@arnetheduck arnetheduck changed the title wip: hashtree Integrate hashtree for better hashing performance Apr 3, 2024
@arnetheduck arnetheduck marked this pull request as ready for review April 3, 2024 10:58
@@ -136,6 +136,10 @@ jobs:
fi
fi

if [[ '${{ runner.os }}' == 'Windows' ]]; then
export NIMFLAGS="${NIMFLAGS} --cc:clang"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this imply new build requisites for https://nimbus.guide/install.html#build-prerequisites ?

Copy link
Member Author

Choose a reason for hiding this comment

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

not really, this is something our ci builds are set up to do explicitly..

Copy link
Member Author

Choose a reason for hiding this comment

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

the problem before this line was that clang pretends to be gcc (via executable name) but isn't compatible enough

@arnetheduck arnetheduck merged commit ab30efa into unstable May 2, 2024
13 checks passed
@arnetheduck arnetheduck deleted the hashtree branch May 2, 2024 06:54
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.

None yet

2 participants