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

fix: bench.rs panics #660

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

Conversation

miningexperiments
Copy link
Contributor

Problem: u128 panics during bench

u128 benches are causing panics. The other Uint128 tests, and others, did not panic, so they were left as is;

e.g.

Benchmarking u128/right shift: Warming up for 3.0000 sthread 'main' panicked at math/benches/bench.rs:50:54:
attempt to shift right with overflow
stack backtrace:
0: 0x613015e7a3aa -
1: 0x613015ce9323 -
2: 0x613015e76f13 -
3: 0x613015e7a202 -
4: 0x613015e7aff7 -
5: 0x613015e7ae5d -
6: 0x613015e7b5a7 -
7: 0x613015e7b416 -
8: 0x613015e7a859 -
9: 0x613015e7b0ec -
10: 0x613015c1ad20 -
11: 0x613015c1b3d7 -
12: 0x613015c830d9 -
13: 0x613015c266ba -
14: 0x613015c48bf5 -
15: 0x613015cadfa4 -
16: 0x613015c7769f -
17: 0x613015ca018d -
18: 0x613015ca18e3 -
19: 0x613015ca4ac9 -
20: 0x613015e72e9d -
21: 0x613015ca1385 -
22: 0x72338162a1ca - __libc_start_call_main
at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
23: 0x72338162a28b - __libc_start_main_impl
at ./csu/../csu/libc-start.c:360:3
24: 0x613015c21ff5 -
25: 0x0 -

error: bench failed, to rerun pass -p kaspa-math --bench bench

Fix:

Since benches are meant to measure performance not correctness, the following u128 benches have added wrapping in order to prevent panics.

  • wrapping_add/sub/mul etc instead of +/-/*...

Testing:

Now I am getting no panics with
cargo bench -p kaspa-math --bench bench

Changed functions to use std::time::Instant which is monotonic, to avoid Rust panics with SystemTime. 

Replaced some unwraps with an expect.

Removed redundant brackets, and secp256k1::
Changed fn clean_old_pending_outpoints to retain keys that are younger than an hour, instead of collecting older than an hour ones as a vector, and then using a new for loop to deleting them.

linting with cargo fmt
# Optimization
flow.rs ; async sync_missing_relay_past_headers

# Fix
- Save memory; changed jobs to pass an iterator to try_join_all instead of collecting a vector and then passing it.

- Changed error check to return an explicit error, instead of implicit
Since benches are meant to measure performance not correctness, the following u128 benches have added wrapping in order to prevent panics. The other Uint128 tests did not panic, so they were left as is;
e.g.
Benchmarking u128/right shift: Warming up for 3.0000 sthread 'main' panicked at math/benches/bench.rs:50:54:
attempt to shift right with overflow
stack backtrace:
   0:     0x613015e7a3aa - <unknown>
   1:     0x613015ce9323 - <unknown>
   2:     0x613015e76f13 - <unknown>
   3:     0x613015e7a202 - <unknown>
   4:     0x613015e7aff7 - <unknown>
   5:     0x613015e7ae5d - <unknown>
   6:     0x613015e7b5a7 - <unknown>
   7:     0x613015e7b416 - <unknown>
   8:     0x613015e7a859 - <unknown>
   9:     0x613015e7b0ec - <unknown>
  10:     0x613015c1ad20 - <unknown>
  11:     0x613015c1b3d7 - <unknown>
  12:     0x613015c830d9 - <unknown>
  13:     0x613015c266ba - <unknown>
  14:     0x613015c48bf5 - <unknown>
  15:     0x613015cadfa4 - <unknown>
  16:     0x613015c7769f - <unknown>
  17:     0x613015ca018d - <unknown>
  18:     0x613015ca18e3 - <unknown>
  19:     0x613015ca4ac9 - <unknown>
  20:     0x613015e72e9d - <unknown>
  21:     0x613015ca1385 - <unknown>
  22:     0x72338162a1ca - __libc_start_call_main
                               at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  23:     0x72338162a28b - __libc_start_main_impl
                               at ./csu/../csu/libc-start.c:360:3
  24:     0x613015c21ff5 - <unknown>
  25:                0x0 - <unknown>

error: bench failed, to rerun pass `-p kaspa-math --bench bench`
to match kaspanet/rusty-kaspa master
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.

1 participant