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

reduce daily block range size to 10 #973

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

reduce daily block range size to 10 #973

wants to merge 2 commits into from

Conversation

edg-l
Copy link
Member

@edg-l edg-l commented Dec 16, 2024

To reduce the timeout chance, daily block running is more a health check, so if it fails due to timeout its pointless

Checklist

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

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.66%. Comparing base (4bd59e9) to head (1bbe4f1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #973   +/-   ##
=======================================
  Coverage   80.66%   80.66%           
=======================================
  Files         107      107           
  Lines       29325    29325           
=======================================
  Hits        23656    23656           
  Misses       5669     5669           

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

Copy link

github-actions bot commented Dec 16, 2024

Benchmarking results

Benchmark for program factorial_2M

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 11.220 ± 0.193 10.750 11.478 8.33 ± 0.14
cairo-native (embedded AOT) 3.348 ± 0.013 3.331 3.370 2.49 ± 0.01
cairo-native (embedded JIT using LLVM's ORC Engine) 3.146 ± 0.016 3.119 3.163 2.34 ± 0.01
cairo-native (standalone AOT with -march=native) 1.346 ± 0.001 1.344 1.348 1.00

Benchmark for program fib_2M

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 10.597 ± 0.321 10.341 11.250 136.56 ± 4.15
cairo-native (embedded AOT) 2.968 ± 0.049 2.901 3.040 38.25 ± 0.64
cairo-native (embedded JIT using LLVM's ORC Engine) 2.733 ± 0.048 2.692 2.858 35.22 ± 0.63
cairo-native (standalone AOT with -march=native) 0.078 ± 0.000 0.077 0.078 1.00

Benchmark for program linear_search

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 4.317 ± 0.038 4.261 4.367 2199.88 ± 47.36
cairo-native (embedded AOT) 3.024 ± 0.016 3.004 3.054 1541.05 ± 31.29
cairo-native (embedded JIT using LLVM's ORC Engine) 2.924 ± 0.015 2.895 2.942 1489.98 ± 30.22
cairo-native (standalone AOT with -march=native) 0.002 ± 0.000 0.002 0.002 1.00

Benchmark for program logistic_map

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 4.197 ± 0.016 4.165 4.223 17.28 ± 0.09
cairo-native (embedded AOT) 3.110 ± 0.029 3.079 3.178 12.80 ± 0.13
cairo-native (embedded JIT using LLVM's ORC Engine) 3.031 ± 0.018 3.004 3.059 12.48 ± 0.09
cairo-native (standalone AOT with -march=native) 0.243 ± 0.001 0.242 0.245 1.00

Copy link

github-actions bot commented Dec 16, 2024

Benchmark results Main vs HEAD.

Command Mean [s] Min [s] Max [s] Relative
base factorial_2M.cairo (JIT) 3.362 ± 0.057 3.262 3.428 1.00
base factorial_2M.cairo (AOT) 3.564 ± 0.083 3.437 3.691 1.06 ± 0.03
Command Mean [s] Min [s] Max [s] Relative
:--- ---: ---: ---: ---:
head factorial_2M.cairo (JIT) 3.210 ± 0.028 3.172 3.271 1.00
head factorial_2M.cairo (AOT) 3.395 ± 0.030 3.358 3.447 1.06 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base fib_2M.cairo (JIT) 2.935 ± 0.067 2.830 3.081 1.00
base fib_2M.cairo (AOT) 3.252 ± 0.031 3.218 3.311 1.11 ± 0.03
Command Mean [s] Min [s] Max [s] Relative
:--- ---: ---: ---: ---:
head fib_2M.cairo (JIT) 2.724 ± 0.022 2.699 2.769 1.00
head fib_2M.cairo (AOT) 2.915 ± 0.012 2.897 2.932 1.07 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base linear_search.cairo (JIT) 3.235 ± 0.033 3.174 3.275 1.00
base linear_search.cairo (AOT) 3.345 ± 0.036 3.284 3.412 1.03 ± 0.02
Command Mean [s] Min [s] Max [s] Relative
:--- ---: ---: ---: ---:
head linear_search.cairo (JIT) 2.938 ± 0.040 2.886 2.998 1.00
head linear_search.cairo (AOT) 3.031 ± 0.029 2.997 3.100 1.03 ± 0.02
Command Mean [s] Min [s] Max [s] Relative
base logistic_map.cairo (JIT) 3.368 ± 0.055 3.290 3.451 1.00
base logistic_map.cairo (AOT) 3.490 ± 0.049 3.418 3.585 1.04 ± 0.02
Command Mean [s] Min [s] Max [s] Relative
:--- ---: ---: ---: ---:
head logistic_map.cairo (JIT) 3.072 ± 0.042 3.002 3.134 1.00
head logistic_map.cairo (AOT) 3.239 ± 0.035 3.204 3.292 1.05 ± 0.02

Copy link
Contributor

@JulianGCalderon JulianGCalderon left a comment

Choose a reason for hiding this comment

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

Right now we are running 40 workflows, with 25 blocks each, summing up to a total of 1000 blocks each day. If we reduce the range size, we should also increase the number of workflows so that we run more or less the same amount of blocks.

Also, maybe a range size of 10 to little, maybe we could try with 20 and see if it finishes in time.

@FrancoGiachetta
Copy link
Contributor

FrancoGiachetta commented Dec 16, 2024

I would say it's preferable to have more blocks and leave it to 10 to be sure it won't fail. Looking at the number of transactions that fail, I think 10 blocks range should have us covered. What do you think?

@JulianGCalderon
Copy link
Contributor

@FrancoGiachetta 10 blocks will probably work, but if it fails with 20 (or 15), we could make another PR lowering it a bit more, right? There is a limit to the amount of jobs that can be run in a workflow, so there is a benefit on trying to find the maximum range size that will run all blocks, just in case later on we want to increase the size of the daily run.

@FrancoGiachetta
Copy link
Contributor

If I'm not mistaken, the limit of workers is 256. But yeah, we could definitely give it a try.

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.

4 participants