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

Feat: Vm Profiling #1789

Open
wants to merge 33 commits into
base: dev
Choose a base branch
from
Open

Conversation

nhtyy
Copy link
Contributor

@nhtyy nhtyy commented Nov 14, 2024

Building on a previous PR #1713

The logic to preserve the callstack is now done during execution (debug only)
and by default the trace file is now a gecko profile json, which can opened via samply load ...

@nhtyy nhtyy force-pushed the n/profiling-in-executor branch 2 times, most recently from e04ad2b to 5e1c1b9 Compare November 14, 2024 00:52
@@ -0,0 +1,24 @@
# Profiling ZKVM programs

Copy link
Contributor

@yuwen01 yuwen01 Nov 14, 2024

Choose a reason for hiding this comment

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

We can either replace the ## Tracking Cycles With Tracing section in book/writing-programs/cycle-tracking.md, or we can move the ## Tracking Cycles With Annotations section from that file here. Either way, we're trying to deprecate the old cargo prove trace thing right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I didnt see that thanks

Copy link
Contributor

@yuwen01 yuwen01 left a comment

Choose a reason for hiding this comment

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

overall looks pretty good, made some small comments about cleaning up docs

To enable profiling, set the `TRACE_FILE` env var to the path where you want the profile to be saved.

The full command to profile should look something like this
```sh
Copy link
Contributor

Choose a reason for hiding this comment

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

as discussed irl: either add RUST_FLAGS or, if the tracing doesn't impact runtime performance too much, get rid of the debug assertions thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RUST_FLAGS doesnt seem to be an option afaict since debug_assertions seems to be owned by cargo

Ok(())
}

/// Simple Check to makes sure we have valid main function that lasts for most of the exeuction
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: caps

crates/core/executor/src/profiler.rs Show resolved Hide resolved
crates/core/executor/src/profiler.rs Show resolved Hide resolved
@nhtyy
Copy link
Contributor Author

nhtyy commented Nov 14, 2024

@yuwen01 the CLI CI test seems to be failing silently here.. Wdyt?

@nhtyy nhtyy force-pushed the n/profiling-in-executor branch 2 times, most recently from 9c5d7fc to 40b61fc Compare November 14, 2024 23:28
Copy link
Contributor

@yuwen01 yuwen01 left a comment

Choose a reason for hiding this comment

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

Left some doc comments, but logic is pretty clear now thanks to your comments. You can check that your changes to the book look good by running mdbook serve from the sp1 root directory


Profiling the ZKVM can only be done with debug builds, and special care must be taken to ensure correctness, only one program may be profiled at a time.

To profile a program, you have to setup a script to execute the program, many examples can be found in the repo, such as this ('fibonacci')[../../examples/fibonacci/script/src/main.rs] script.
Copy link
Contributor

Choose a reason for hiding this comment

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

link broken


```bash
TRACE_FILE=trace.log RUST_LOG=info cargo run --release
To profile a program, you have to setup a script to execute the program, many examples can be found in the repo, such as this ('fibonacci')[../../examples/fibonacci/script/src/main.rs] script.
Copy link
Contributor

Choose a reason for hiding this comment

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

To profile a program, you have to setup a script to execute the program. Many examples can be found in the repo, such as this ['fibonacci'](../../examples/fibonacci/script/src/main.rs) script.


The `cycle-tracker` annotation is a convenient way to track cycles for specific sections of code. However, sometimes it can also be useful to track what functions are taking the most cycles across the entire program, without having to annotate every function individually.
Profiling the VM is a good way to get an understanding of what is bottlenecking your program, and special care must be taken to ensure correctness, only one program may be profiled at a time.
Copy link
Contributor

Choose a reason for hiding this comment

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

Profiling the zkVM is a good way to get an understanding of what is bottlenecking your program. Note that only one program may be profiled at a time.

When the `TRACE_FILE` environment variable is set, as SP1's RISC-V runtime is executing, it will write a log of the program counter to the file specified by `TRACE_FILE`.

Next, we can use the `cargo prove` CLI with the `trace` command to analyze the trace file and generate a table of instruction counts. This can be done with the following command:
The data captured by the profiler can be quite large, you can set the sample rate using the `TRACE_SAMPLE_RATE` env var.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain that a higher sample rate will result in smaller files? Someone might think that a small rate will decrease the sampling frequency

| syscall_hint_read | 3 |
+----------------------------------------+-------------------+

To profile in release mode you can use the following setup in your `Cargo.toml`
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this no longer necessary

let start_address = sym.st_value;
let end_address = start_address + size - 4;

// now that we have the name lets immeidalty intern it so we only need to copy
Copy link
Contributor

Choose a reason for hiding this comment

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

typo, also you should use complete sentences and capitalization.


/// The ZKVM Profiler.
///
/// During exeuction, the profiler always keeps track of the callstack
Copy link
Contributor

Choose a reason for hiding this comment

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

exeuction -> execution

```

The `trace` command will generate a table of instruction counts, sorted by the number of cycles spent in each function. The output will look something like this:

To view these profiles, we recommend (Samply)[https://github.com/mstange/samply].
Copy link
Contributor

Choose a reason for hiding this comment

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

broken link

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.

2 participants