Conversation
5d18c01 to
aa33bf1
Compare
76b8508 to
4432551
Compare
aa33bf1 to
45feb8b
Compare
4432551 to
3135903
Compare
45feb8b to
5c26e30
Compare
3135903 to
fe48470
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45feb8b392
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5c26e30 to
7e37cd6
Compare
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on orizi and TomerStarkware).
tests/benches/compile.rs line 52 at r2 (raw file):
// Phase: source → Sierra (full IR generation). group.bench_function("cairo-to-sierra", |b| {
Dont we need separate function names for differrent benchmarks?
tests/benches/compile.rs line 189 at r2 (raw file):
} criterion_group!(benches, bench_compile);
Wont this give
orizi
left a comment
There was a problem hiding this comment.
@orizi made 2 comments.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on eytan-starkware and TomerStarkware).
tests/benches/compile.rs line 52 at r2 (raw file):
Previously, eytan-starkware wrote…
Dont we need separate function names for differrent benchmarks?
the groups seems to be enough.
tests/benches/compile.rs line 189 at r2 (raw file):
Previously, eytan-starkware wrote…
Wont this give
?
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware made 1 comment and resolved 1 discussion.
Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on orizi and TomerStarkware).
tests/benches/compile.rs line 52 at r2 (raw file):
Previously, orizi wrote…
the groups seems to be enough.
I am thinking it will be better to see for each phase how each benchmark behaved, currently it will be hard to compare a slowdown in phase x, as we will need to compare 3 different tables
7e37cd6 to
9d7fce7
Compare
9d7fce7 to
1157d46
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on eytan-starkware and TomerStarkware).
tests/benches/compile.rs line 52 at r2 (raw file):
Previously, eytan-starkware wrote…
I am thinking it will be better to see for each phase how each benchmark behaved, currently it will be hard to compare a slowdown in phase x, as we will need to compare 3 different tables
i flipped it now.
eytan-starkware
left a comment
There was a problem hiding this comment.
@eytan-starkware reviewed 3 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).

TL;DR
Added a new benchmark that measures compilation performance of the corelib itself, alongside the existing
fib.cairobenchmark.What changed?
The benchmark infrastructure was refactored to extract reusable
build_dbandbuild_test_dbhelper functions, eliminating repeatedRootDatabasebuilder boilerplate. The benchmark phases (cairo-to-sierra, cairo-to-diagnostics, cairo-to-testing, cairo-to-cache, cache-to-sierra, cache-to-testing) were extracted into a genericbench_all_phasesfunction that accepts a path, group name, label, and adetect_coreflag.A new
bench_compile_corelibbenchmark was added that runs all the same phases against thecorelibdirectory (withdetect_core: false), and bothbench_compileandbench_compile_corelibare now registered withcriterion_group!.How to test?
Run the benchmarks with:
Both the
compileandcompile_corelibbenchmark groups should appear in the output.Why make this change?
Benchmarking only a small example file like
fib.cairodoes not capture the performance characteristics of compiling a large, real-world codebase. Adding corelib as a benchmark target provides a more representative and meaningful measure of compiler performance at scale.