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.

Sign up for GitHub

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

Jump to bottom

Address some performance regressions #4720

Merged
merged 3 commits into from
Sep 23, 2020
Merged

Address some performance regressions #4720

merged 3 commits into from
Sep 23, 2020

Conversation

ebfull
Copy link
Contributor

@ebfull ebfull commented Sep 9, 2020 •
edited
Loading

@str4d identified and fixed some performance regressions in our elliptic curve and proving crates, and we also changed to complete formulas in bls12_381 to get some better performance in some cases. The result:

v3.1.0
"runningtime": 0.59883

before upgrading rust:
"runningtime": 0.823932

after upgrading rust:
"runningtime": 0.83004

after upgrading rust packages:
"runningtime": 0.763065

which gets us some of the way back to where we were.

@zkbot
Copy link
Contributor

zkbot commented Sep 18, 2020

☔ The latest upstream changes (presumably #4732) made this pull request unmergeable. Please resolve the merge conflicts.

@str4d str4d added A-dependencies Area: Dependencies I-performance Problems and improvements with respect to performance labels Sep 19, 2020
@str4d
Copy link
Contributor

str4d commented Sep 19, 2020

This needs to be rebased. The changes to depends/ will all be removed as a result of #4725.

@ebfull ebfull force-pushed the fix-perf-regressions branch from 390a298 to 52777e6 Compare September 22, 2020 21:15
str4d
str4d approved these changes Sep 23, 2020
View reviewed changes
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

ACK. I verified the hashes of the native_rust source tarballs.

@str4d
Copy link
Contributor

str4d commented Sep 23, 2020

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Sep 23, 2020

📌 Commit 52777e6 has been approved by str4d

@zkbot
Copy link
Contributor

zkbot commented Sep 23, 2020

⌛ Testing commit 52777e6 with merge 49fc20a...

@zkbot
Copy link
Contributor

zkbot commented Sep 23, 2020

☀️ Test successful - pr-merge
Approved by: str4d
Pushing 49fc20a to master...

@zkbot zkbot merged commit 49fc20a into zcash:master Sep 23, 2020
This was referenced Sep 23, 2020
Draft
Closed
Merged
@str4d str4d mentioned this pull request Sep 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@str4d str4d str4d approved these changes

Assignees
No one assigned
Labels
A-dependencies Area: Dependencies I-performance Problems and improvements with respect to performance
Projects
None yet
Milestone
Core Sprint 2020-37
Development

Successfully merging this pull request may close these issues.
4 participants
@ebfull @zkbot @str4d @r3ld3v