succinctlabs / sp1

The fastest, most feature-complete zkVM for developers.
https://succinctlabs.github.io/sp1
Apache License 2.0
1.02k stars 334 forks source link

perf: Strip the RISC-V ELF binary #1806

Closed imikushin closed 5 days ago

imikushin commented 5 days ago

This reduces the size of produced binaries by more than 2x.

Motivation

The generated ELF binaries are pretty large: the fibonacci example from Quickstart compiles to a 118KB binary. When looking at it with the file utility, one can see that it's not stripped.

❯ la $SP1_ELF_fibonacci_program
-rwxr-xr-x@ 1 ivan  staff   118K Nov 20 16:14 /Users/ivan/src/succinct/fibonacci/target/elf-compilation/riscv32im-succinct-zkvm-elf/release/fibonacci-program

❯ file $SP1_ELF_fibonacci_program
/Users/ivan/src/succinct/fibonacci/target/elf-compilation/riscv32im-succinct-zkvm-elf/release/fibonacci-program: ELF 32-bit LSB executable, UCB RISC-V, soft-float ABI, version 1 (SYSV), statically linked, not stripped

Solution

Adding "-C", "strip=symbols", "-C", "opt-level=z" to Rust compiler flags reduces the produced ELF binary size by more than 2x.

❯ la $SP1_ELF_fibonacci_program
-rwxr-xr-x@ 1 ivan  staff    53K Nov 20 16:20 /Users/ivan/src/succinct/fibonacci/target/elf-compilation/riscv32im-succinct-zkvm-elf/release/fibonacci-program

❯ file $SP1_ELF_fibonacci_program
/Users/ivan/src/succinct/fibonacci/target/elf-compilation/riscv32im-succinct-zkvm-elf/release/fibonacci-program: ELF 32-bit LSB executable, UCB RISC-V, soft-float ABI, version 1 (SYSV), statically linked, stripped

PR Checklist

matthiasgoergens commented 5 days ago

What are you trying to optimise for? What do we gain out of having a smaller binary?

If it's specifically for prover performance: you should look at the amount of data the prover has to handle, and that's not directly related to ELF binary size: as far as I can tell, debug symbols are already ignored by the prover.

About the opt-level: you should specifically check whether going for 'z' makes your proof take longer than eg opt-level 2 or 3, if you care about that sort of thing.

imikushin commented 5 days ago

@matthiasgoergens This change is simply to make the produced binary size smaller: it seems that I'll need store these binaries, and making them 2x smaller will save 2x the cost — it's that simple. Stripping them of symbols doesn't seem to hurt neither prover nor verifier while cutting unnecessary waste.

matthiasgoergens commented 5 days ago

Thanks for the reply.

There are two things happening in your PR:

  1. You strip the debug symbols. You are right that neither prover nor verifier care about those. It's just a bit annoying for debuggability. (We just had an occasion at my work, where having the debug symbols available proved really useful.)
  2. You change the optimisation level to 'z'. That will have an impact on prover performance. It can save you a few bytes that the prover doesn't have to load, but it might cost you more VM cycles that need proving. I would suggest doing benchmarks of your individual guest programs, to determine whether that's a good trade-off.

There might be some circumstances where you want to save every last byte of the binary, even at the cost of debuggability or having extra VM CPU cycles that need proving. But my guess is that those circumstances are rare enough, that you wouldn't want to enable the flags you suggested by default for everyone under all circumstances.

However, keep in mind that I'm not associated with Succinct or SP1. So this is just my opinion, and doesn't reflect anything 'official'.

Btw, if you want to improve the default build configuration for guest programs, I would suggest enabling link time optimisation. That alone saved us 7% of cycles in our project.

imikushin commented 5 days ago

Actually, there's a workaround for this, without making this change: simply add these flags to Cargo.toml of the project (workspace) building the RISC-V binary.

[profile.release]
opt-level = "z"
strip = "symbols"

So, feel free to close if this isn't adding value.

matthiasgoergens commented 5 days ago

@imikushin You bring up a great point! I might be useful to stick your suggestion (together with an explanation of when one might want to do this) in a README somewhere in https://github.com/succinctlabs/sp1-project-template or as a comment inside the generated Cargo.toml in that template.

Btw, you can add lto="fat" there, too. (You should try it out for your own project, or for the examples.)

imikushin commented 5 days ago

Closing in favor of succinctlabs/sp1-project-template#42