Skip to content

Add compilation stats #1339

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

Open
wants to merge 44 commits into
base: main
Choose a base branch
from
Open

Add compilation stats #1339

wants to merge 44 commits into from

Conversation

DiegoCivi
Copy link
Contributor

@DiegoCivi DiegoCivi commented Jul 31, 2025

We have contracts that take quite a bit of time to compile. To understand why this is happening we want to add more compilation stats.

Added stats:

  • sierra_declared_types_stats: Contains the stats for each Sierra declared type
    • size: Size of the type
    • as_param_count: Number of times the type is used as a parameter in a libfunc
  • sierra_func_stats: Stats about params and return types of each Sierra function
  • sierra_circuit_gates_count: Number of times each circuit gate is used

Checklist

  • Linked to Github Issue
  • Unit tests added
  • Integration tests added.
  • This change requires new documentation.
    • Documentation has been added/updated.

Copy link

github-actions bot commented Jul 31, 2025

Benchmark results Main vs HEAD.

Base

Command Mean [s] Min [s] Max [s] Relative
base dict_insert.cairo (JIT) 5.239 ± 0.048 5.180 5.311 1.02 ± 0.01
base dict_insert.cairo (AOT) 5.159 ± 0.045 5.110 5.247 1.00

Head

Command Mean [s] Min [s] Max [s] Relative
head dict_insert.cairo (JIT) 5.346 ± 0.063 5.244 5.432 1.01 ± 0.02
head dict_insert.cairo (AOT) 5.296 ± 0.051 5.215 5.372 1.00

Base

Command Mean [s] Min [s] Max [s] Relative
base dict_snapshot.cairo (JIT) 5.042 ± 0.059 4.969 5.172 1.04 ± 0.02
base dict_snapshot.cairo (AOT) 4.871 ± 0.055 4.795 4.964 1.00

Head

Command Mean [s] Min [s] Max [s] Relative
head dict_snapshot.cairo (JIT) 5.058 ± 0.039 5.004 5.116 1.03 ± 0.01
head dict_snapshot.cairo (AOT) 4.916 ± 0.042 4.845 4.990 1.00

Base

Command Mean [s] Min [s] Max [s] Relative
base factorial_2M.cairo (JIT) 5.290 ± 0.042 5.236 5.356 1.00 ± 0.01
base factorial_2M.cairo (AOT) 5.272 ± 0.024 5.220 5.299 1.00

Head

Command Mean [s] Min [s] Max [s] Relative
head factorial_2M.cairo (JIT) 5.362 ± 0.035 5.314 5.419 1.00
head factorial_2M.cairo (AOT) 5.420 ± 0.043 5.371 5.498 1.01 ± 0.01

Base

Command Mean [s] Min [s] Max [s] Relative
base fib_2M.cairo (JIT) 4.858 ± 0.028 4.819 4.906 1.01 ± 0.01
base fib_2M.cairo (AOT) 4.830 ± 0.054 4.743 4.907 1.00

Head

Command Mean [s] Min [s] Max [s] Relative
head fib_2M.cairo (JIT) 4.965 ± 0.040 4.930 5.066 1.00
head fib_2M.cairo (AOT) 4.970 ± 0.086 4.864 5.161 1.00 ± 0.02

Base

Command Mean [s] Min [s] Max [s] Relative
base linear_search.cairo (JIT) 5.033 ± 0.033 4.992 5.103 1.02 ± 0.01
base linear_search.cairo (AOT) 4.928 ± 0.037 4.879 4.984 1.00

Head

Command Mean [s] Min [s] Max [s] Relative
head linear_search.cairo (JIT) 5.304 ± 0.034 5.252 5.356 1.03 ± 0.01
head linear_search.cairo (AOT) 5.146 ± 0.049 5.059 5.226 1.00

Base

Command Mean [s] Min [s] Max [s] Relative
base logistic_map.cairo (JIT) 5.111 ± 0.022 5.077 5.148 1.03 ± 0.01
base logistic_map.cairo (AOT) 4.949 ± 0.034 4.910 5.002 1.00

Head

Command Mean [s] Min [s] Max [s] Relative
head logistic_map.cairo (JIT) 5.295 ± 0.065 5.202 5.394 1.04 ± 0.01
head logistic_map.cairo (AOT) 5.070 ± 0.025 5.040 5.123 1.00

Copy link

github-actions bot commented Jul 31, 2025

Benchmarking results

Benchmark for program dict_insert

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 11.608 ± 0.080 11.505 11.754 2.18 ± 0.03
cairo-native (embedded AOT) 5.317 ± 0.071 5.234 5.462 1.00
cairo-native (embedded JIT using LLVM's ORC Engine) 5.416 ± 0.047 5.315 5.480 1.02 ± 0.02

Benchmark for program dict_snapshot

Open benchmarks
Command Mean [ms] Min [ms] Max [ms] Relative
Cairo-vm (Rust, Cairo 1) 879.5 ± 13.2 860.0 899.6 1.00
cairo-native (embedded AOT) 5152.8 ± 70.8 5043.4 5259.7 5.86 ± 0.12
cairo-native (embedded JIT using LLVM's ORC Engine) 5170.2 ± 78.2 5038.6 5287.7 5.88 ± 0.13

Benchmark for program factorial_2M

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 5.387 ± 0.046 5.315 5.466 1.00
cairo-native (embedded AOT) 5.479 ± 0.081 5.390 5.635 1.02 ± 0.02
cairo-native (embedded JIT using LLVM's ORC Engine) 5.511 ± 0.043 5.436 5.577 1.02 ± 0.01

Benchmark for program fib_2M

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 5.183 ± 0.061 5.130 5.347 1.05 ± 0.02
cairo-native (embedded AOT) 4.918 ± 0.047 4.829 4.979 1.00
cairo-native (embedded JIT using LLVM's ORC Engine) 4.966 ± 0.070 4.876 5.071 1.01 ± 0.02

Benchmark for program linear_search

Open benchmarks
Command Mean [ms] Min [ms] Max [ms] Relative
Cairo-vm (Rust, Cairo 1) 858.4 ± 14.8 838.1 875.4 1.00
cairo-native (embedded AOT) 5017.4 ± 78.7 4913.2 5120.7 5.84 ± 0.14
cairo-native (embedded JIT using LLVM's ORC Engine) 5235.3 ± 93.9 5073.7 5360.2 6.10 ± 0.15

Benchmark for program logistic_map

Open benchmarks
Command Mean [ms] Min [ms] Max [ms] Relative
Cairo-vm (Rust, Cairo 1) 558.8 ± 6.4 552.0 570.5 1.00
cairo-native (embedded AOT) 5097.5 ± 56.2 5001.5 5189.4 9.12 ± 0.15
cairo-native (embedded JIT using LLVM's ORC Engine) 5313.8 ± 51.5 5241.8 5384.3 9.51 ± 0.14

@codecov-commenter
Copy link

codecov-commenter commented Jul 31, 2025

Codecov Report

❌ Patch coverage is 0% with 159 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.81%. Comparing base (88a5f54) to head (ceeb6a2).

Files with missing lines Patch % Lines
src/executor/contract.rs 0.00% 60 Missing ⚠️
src/statistics.rs 0.00% 47 Missing ⚠️
src/debug.rs 0.00% 40 Missing ⚠️
src/utils.rs 0.00% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1339      +/-   ##
==========================================
- Coverage   79.16%   78.81%   -0.36%     
==========================================
  Files         111      111              
  Lines       30440    30578     +138     
==========================================
  Hits        24099    24099              
- Misses       6341     6479     +138     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@DiegoCivi DiegoCivi marked this pull request as ready for review August 5, 2025 20:49
@JulianGCalderon
Copy link
Contributor

I don't like the idea of saving max values as statistics. Maybe a contract can have one really big function and a lot of small functions, while other contract have a lot of medium functions. The max value wouldn't be too significant. I would try to save the full data in every case, and not some aggregated value. If this ends up being too much data, we could have at least a more accurate representation of the distribution (like some percentiles/quartiles).

Is there a way to link declaration with usage? For example:

  • Maybe a type is really big but its only used in a single libfunc, whereas a medium-sized type is used in a lot of libfuncs.
  • Idem for a function declaration. Maybe a big signature function is called once, whereas a medium-sized signature function is called a lot of times.

Is there a way to record this information?

We need to find some value/set of values that predicts with acceptable accuracy the compilation effort, and I feel like that involves "combining" different metrics together.

What do you think?

let type_id = type_declaration.id.to_string();
let type_size = type_concrete.layout(&registry).unwrap().size();
if !type_concrete.is_builtin() {
// We dont want to add the builtins to the stats
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are builtins skipped?

Copy link
Contributor Author

@DiegoCivi DiegoCivi Aug 18, 2025

Choose a reason for hiding this comment

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

I figured they are always the same size. So I though it didn't make much sense to have them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Its not worth treating them any differently. There are many types that are always the same size (i.e. u8). I would keep them, it would also make the code a bit simpler.

@@ -416,6 +423,20 @@ pub fn layout_repeat(layout: &Layout, n: usize) -> Result<(Layout, usize), Layou
Ok((layout, padded_size))
}

/// Gets the size of the full set of params of a Sierra function
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we can use any type, not just the params of a function, so we could update the documentation to be more generic.

Suggested change
/// Gets the size of the full set of params of a Sierra function
/// Returns the total layout size for the given types.

Comment on lines +56 to +57
/// - params_total_size: Total size of all the params
/// - return_types_total_size: Total size of all the params
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move each bullet point to the appropiate field doc comment. Idem for the other struct.

Something like:

/// Contains the following stats about a Sierra function:
pub struct SierraFuncStats {
    /// Total size of all the params
    pub params_total_size: usize,
    /// Total size of all the params
    pub return_types_total_size: usize,
    pub times_used: usize,
}

Comment on lines +25 to +26
/// Number of times each circuit gate is used.
sierra_circuit_gates_count: CircuitGatesStats,
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this field be public?

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.

3 participants