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

Improve speed and accuracy of zcbenchmark validatelargetx #1194

Merged

Conversation

bitcartel
Copy link
Contributor

@bitcartel bitcartel commented Aug 8, 2016

The verification test, in a loop, passes spending_tx (a CMutableTransaction) to the constructor of MutableTransactionSignatureChecker, which immediately uses it to create a non-mutable CTransaction object, which is used for the actual verification process.

Since spending_tx is not mutated during the verification loop & process, we can instead convert it to a CTransaction just once, and use it with TransactionSignatureChecker.

This removes the time to create CTransaction objects from the benchmark itself.

Results show an improvement in running time to complete the verification phase of the test and consistent times across z7 and z8 releases.

Benchmarks on i3 processor:
z7 old 228.67205900 --> z7 new 49.27225200
z7 old 229.90048900 --> z7 new 48.38650700
z8 old 295.77963800 --> z8 new 48.37695100
z8 old 294.32640100 --> z8 new 49.93216100

unncessarily create thousands of CTransaction objects.
@bitcartel bitcartel added I-performance Problems and improvements with respect to performance A-testing Area: Tests and testing infrastructure labels Aug 8, 2016
@bitcartel bitcartel self-assigned this Aug 8, 2016
@bitcartel bitcartel added this to the z9 Release - RPC, Audit tasks, Testing improvements milestone Aug 8, 2016
@bitcartel bitcartel added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 8, 2016
@bitcartel bitcartel changed the title Improve accuracy of zcbenchmark validatelargetx Improve speed and accuracy of zcbenchmark validatelargetx Aug 8, 2016
@daira
Copy link
Contributor

daira commented Aug 8, 2016

ACK

1 similar comment
@ebfull
Copy link
Contributor

ebfull commented Aug 8, 2016

ACK

@ebfull
Copy link
Contributor

ebfull commented Aug 8, 2016

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Aug 8, 2016

📌 Commit 75c2f26 has been approved by ebfull

@zkbot
Copy link
Contributor

zkbot commented Aug 8, 2016

⌛ Testing commit 75c2f26 with merge 75c2f26...

@zkbot
Copy link
Contributor

zkbot commented Aug 9, 2016

💥 Test timed out

@bitcartel
Copy link
Contributor Author

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented Aug 9, 2016

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.

@zkbot
Copy link
Contributor

zkbot commented Aug 9, 2016

📌 Commit 75c2f26 has been approved by bitcartel

@bitcartel
Copy link
Contributor Author

@zkbot retry

@zkbot
Copy link
Contributor

zkbot commented Aug 9, 2016

⌛ Testing commit 75c2f26 with merge 75c2f26...

@zkbot
Copy link
Contributor

zkbot commented Aug 9, 2016

💥 Test timed out

@ebfull
Copy link
Contributor

ebfull commented Aug 14, 2016

@zkbot retry

@ebfull
Copy link
Contributor

ebfull commented Aug 14, 2016

@zkbot r+

@ebfull
Copy link
Contributor

ebfull commented Aug 14, 2016

@zkbot r+ 75c2f26

@zkbot
Copy link
Contributor

zkbot commented Aug 14, 2016

🙀 75c2f2688744c9b63fee1a87dfc01ab49ac289b6 is not a valid commit SHA. Please try again with 998eea5.

@ebfull
Copy link
Contributor

ebfull commented Aug 14, 2016

@zkbot r+ 75c2f26

@zkbot
Copy link
Contributor

zkbot commented Aug 14, 2016

⌛ Testing commit 75c2f26 with merge d7da4ec...

zkbot pushed a commit that referenced this pull request Aug 14, 2016
…bfull

Improve speed and accuracy of zcbenchmark validatelargetx

The verification test, in a loop, passes `spending_tx` (a `CMutableTransaction`) to the constructor of `MutableTransactionSignatureChecker`, which immediately uses it to create a non-mutable `CTransaction` object, which is used for the actual verification process.

Since `spending_tx` is not mutated during the verification loop & process, we can instead convert it to a `CTransaction` just once, and use it with `TransactionSignatureChecker`.

This removes the time to create `CTransaction` objects from the benchmark itself.

Results show an improvement in running time to complete the verification phase of the test and consistent times across z7 and z8 releases.

```
Benchmarks on i3 processor:
z7 old 228.67205900 --> z7 new 49.27225200
z7 old 229.90048900 --> z7 new 48.38650700
z8 old 295.77963800 --> z8 new 48.37695100
z8 old 294.32640100 --> z8 new 49.93216100
```
@zkbot
Copy link
Contributor

zkbot commented Aug 14, 2016

☀️ Test successful - zcash

@zkbot zkbot merged commit 75c2f26 into zcash:zc.v0.11.2.latest Aug 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing Area: Tests and testing infrastructure I-performance Problems and improvements with respect to performance S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants