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

Add soroban-bench-utils, add benchmark tests to measure metering accuracy #956

Merged
merged 13 commits into from
Jul 25, 2023

Conversation

jayz22
Copy link
Contributor

@jayz22 jayz22 commented Jul 18, 2023

What

Resolves #710
This PR moves the instrumentation helpers from the bench to the host (only turned on for linux and macos), and added a new helper class HostTracker to get the actual resource usage.

Added two test contracts and run them with instrumentation turned on, here are the results comparing modeled vs actual usage:


$RUST_TEST_THREADS=1  cargo test --release --package soroban-env-host --lib -- test::metering_benchmark  --nocapture --ignored

running 2 tests
test test::metering_benchmark::run_add_i32 ... 
model cpu consumed: 1138241, actual cpu insns 1452320 
model mem consumed: 1274466, actual mem bytes 1213168
ok
test test::metering_benchmark::run_complex ... 
model cpu consumed: 1618593, actual cpu insns 1822004 
model mem consumed: 1308496, actual mem bytes 1244696
ok

Which looks decent, but the difference is hard to tell due to VmInstantiation taking up majority of the share.

Why

[TODO: Why this change is being made. Include any context required to understand the why.]

Known limitations

  1. The tracking_allocator dependency has been moved from dev to actual. Because the HostTracker is used for "bench" as well (which is under feature testutils, but still needs the dependency). Alternatively we make mod instrumentation a separate package and use it as a dev-dependency. I've moved the tracking-related utils to a separate package soroban-bench-utils to avoid the prod dependency.
  2. These tests are disabled by default. For two reasons (added a comment in the test file):
    1. They need to be run with release profile.
    2. They cannot be run with multiple threads due to the gloabal_tracker contention.
  3. Only supports os="linux" and "macos", due to the cpu instruction counting library being only available there. The "other" os now has a stub InstructionCounter that returns 0 always.

@jayz22 jayz22 requested a review from a team as a code owner July 18, 2023 22:14
@jayz22 jayz22 changed the title Move instrumentation helpers to the host, add metering benchmark tests Add soroban-bench-utils, add benchmark tests to measure metering accuracy Jul 18, 2023
Copy link
Contributor

@graydon graydon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@sisuresh sisuresh merged commit 806e38d into stellar:main Jul 25, 2023
8 checks passed
@jayz22 jayz22 deleted the instrument branch July 25, 2023 16:19
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.

Integration testing for metering coverage
3 participants