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.

Sign up for GitHub

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

Jump to bottom

[chore] Strip debug_info when not collecting metrics #865

Merged
merged 1 commit into from
Nov 26, 2024
Merged

[chore] Strip debug_info when not collecting metrics #865

merged 1 commit into from
Nov 26, 2024

Conversation

nyunyunyunyu
Copy link
Contributor

@nyunyunyunyu nyunyunyunyu commented Nov 25, 2024 •
edited
Loading

@jonathanpwang
Copy link
Contributor

is this for eDSL or rust? in rust when we compile with --profile maxperf it already strips the ELF itself

@nyunyunyunyu
Copy link
Contributor Author

is this for eDSL or rust? in rust when we compile with --profile maxperf it already strips the ELF itself

For eDSL, wanna do a quick benchmark to see how much will this gain.

@github-actions GitHub Actions
Copy link

Benchmarks

group app_log_blowup app_total_cells_used app_total_cycles app_total_proof_time_ms leaf_log_blowup leaf_total_cells_used leaf_total_cycles leaf_total_proof_time_ms instance alloc
base64_json_program
2
(+14,572 [+0.1%])
14,996,988
(+42 [+0.0%])
217,352
(+10.0 [+0.4%])
2,534.0
-
-
-
-
64cpu-linux-arm64 mimalloc
fibonacci_program
2
(-4,740 [-0.0%])
51,512,218
1,500,219
(+17.0 [+0.3%])
6,399.0
-
-
-
-
64cpu-linux-arm64 mimalloc
fibonacci_program
2
(-2,376 [-0.0%])
51,512,218
1,500,219
(-832.0 [-11.6%])
6,368.0
-
-
-
-
64cpu-linux-x64 jemalloc
regex_program
2
(+15,167 [+0.0%])
238,565,138
(+22 [+0.0%])
4,181,072
(-525.0 [-1.9%])
26,653.0
-
-
-
-
64cpu-linux-arm64 mimalloc
verify_fibair
2
8,306,107
199,142
(-66.0 [-4.5%])
1,402.0
-
-
-
-
64cpu-linux-arm64 mimalloc

E2E Benchmarks

group app_log_blowup app_total_cells_used app_total_cycles app_total_proof_time_ms leaf_log_blowup leaf_total_cells_used leaf_total_cycles leaf_total_proof_time_ms root_log_blowup root_total_cells_used root_total_cycles root_total_proof_time_ms internal_log_blowup internal_total_cells_used internal_total_cycles internal_total_proof_time_ms instance alloc
fibonacci_continuation_program
2
12,161,126
12,000,219
37,904.0
2
144,064,363
3,638,938
73,298.0
2
987,275,149
24,126,875
94,866.0
2
858,903,165
21,779,507
83,740.0
64cpu-linux-arm64 mimalloc

Commit: 70ef0c5

Benchmark Workflow

@nyunyunyunyu nyunyunyunyu marked this pull request as ready for review November 26, 2024 03:40
@nyunyunyunyu nyunyunyunyu enabled auto-merge (squash) November 26, 2024 05:46
jonathanpwang
jonathanpwang reviewed Nov 26, 2024
View reviewed changes
vm/src/arch/segment.rs
@@ -90,6 +90,11 @@ impl<F: PrimeField32> ExecutionSegment<F> {
) -> Self {
let mut chip_set = config.create_chip_set();
chip_set.set_streams(streams);
let program = if config.collect_metrics {
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 update new_segment as well?

jonathanpwang
jonathanpwang approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@jonathanpwang jonathanpwang left a comment

Choose a reason for hiding this comment

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

hmm can you explain why this makes such a big difference? Is it because there's some cloning?

@nyunyunyunyu nyunyunyunyu merged commit 61a772e into main Nov 26, 2024
19 of 24 checks passed
@nyunyunyunyu nyunyunyunyu deleted the chore/exe-without-debug branch November 26, 2024 21:52
@nyunyunyunyu
Copy link
Contributor Author

hmm can you explain why this makes such a big difference? Is it because there's some cloning?

get_instruction clones debug_info

nyunyunyunyu added a commit that referenced this pull request Nov 26, 2024
@nyunyunyunyu
Revert "Strip debug_info when not collecting metrics (#865)"
8c25b82 
This reverts commit 61a772e.
@nyunyunyunyu nyunyunyunyu mentioned this pull request Nov 26, 2024
Merged
luffykai pushed a commit that referenced this pull request Dec 13, 2024
@nyunyunyunyu
Strip debug_info when not collecting metrics (#865)
71edd35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanpwang jonathanpwang jonathanpwang approved these changes

@zlangley zlangley Awaiting requested review from zlangley

Assignees
No one assigned
Labels
run-benchmark-e2e
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nyunyunyunyu @jonathanpwang