-
Notifications
You must be signed in to change notification settings - Fork 1k
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 crypto #3174
Optimize crypto #3174
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and a simple change.
Can you check @superboyiii ?
This is already tested by @superboyiii . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks similar to ECPointCache
, but at another level. Not sure why this can be a problem for C# (it takes time to calculate Y, here you already have it), but if it works (in a size-limited fashion of course) and gives measurable difference, then OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use FIFOCache instead of regular dictionary?
We can change it to FIFOCache with a large limit. If the limit was small, the performance maybe more lower than no cache. |
To me it's still very, very similar to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@superboyiii ,can you check the performance difference when syncing?
If it is 25% as described we can merge now and later modify with other suggestions. |
Its 25% - 30% performance increase from the benchmark. But its for syncing the chain. |
Very good,did u also replicate this result? |
I found a bug in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests failing, can't be merged.
The test checked that the bug was fine xD |
Yes, it's almost 30% speed up. |
Description
Cahced crypto object could optimize verify signature performance。
Fixes # (no related)
Type of change
How Has This Been Tested?
Syncing blocks speed up about 25%.
Checklist: