Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

perf: move arg for to_cairo_runner_program #793

Merged
merged 2 commits into from
Jul 11, 2023

Conversation

Oppen
Copy link
Contributor

@Oppen Oppen commented Jul 7, 2023

Replaces a clone for the program argument to to_cairo_runner_program by a move.
A simple benchmark was performed in our server by reproducing 200 transactions with Pathfinder 5 times. The win is ~10%.

Benchmark Results

[root@archlinux-latest-64-minimal pathfinder]# for i in {1..5}; do time ./re_execute_blockifier ./mainnet_83389.sqlite 68000 68200; done > /dev/null
                                                           
real    0m20.799s 
user    9m54.753s                              
sys     0m22.870s

real    0m20.649s
user    9m53.784s
sys     0m21.372s

real    0m20.685s
user    9m55.955s
sys     0m21.103s

real    0m20.688s
user    9m54.542s
sys     0m21.005s

real    0m20.714s
user    9m55.268s
sys     0m20.871s

[root@archlinux-latest-64-minimal pathfinder]# for i in {1..5}; do time ./re_execute_sir_base ./mainnet_83389.sqlite 68000 68200; done > /dev/null

real    0m33.326s
user    16m12.373s
sys     0m35.381s

real    0m33.320s
user    16m12.506s
sys     0m35.555s

real    0m33.402s
user    16m14.239s
sys     0m35.048s

real    0m33.365s
user    16m13.138s
sys     0m35.427s

real    0m33.282s
user    16m10.810s
sys     0m35.725s

[root@archlinux-latest-64-minimal pathfinder]# for i in {1..5}; do time ./re_execute_sir_owned ./mainnet_83389.sqlite 68000 68200; done > /dev/null

real    0m29.778s
user    14m36.011s
sys     0m24.367s

real    0m29.803s
user    14m35.188s
sys     0m24.706s
  
real    0m29.858s
user    14m35.515s
sys     0m24.574s

real    0m29.837s
user    14m35.700s
sys     0m24.613s

real    0m29.662s
user    14m31.473s
sys     0m24.144s

Checklist

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

@Oppen Oppen marked this pull request as ready for review July 11, 2023 12:48
Copy link
Contributor

@MegaRedHand MegaRedHand left a comment

Choose a reason for hiding this comment

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

🚀 🚀

@codecov-commenter
Copy link

Codecov Report

Merging #793 (059c513) into main (dda6001) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #793      +/-   ##
==========================================
- Coverage   93.18%   93.18%   -0.01%     
==========================================
  Files          52       52              
  Lines       12146    12145       -1     
==========================================
- Hits        11318    11317       -1     
  Misses        828      828              
Impacted Files Coverage Δ
.../api/contract_classes/deprecated_contract_class.rs 86.53% <100.00%> (-0.06%) ⬇️

@SantiagoPittella SantiagoPittella added this pull request to the merge queue Jul 11, 2023
Merged via the queue into main with commit c8b8a70 Jul 11, 2023
@pefontana pefontana deleted the perf/owned_to_program branch July 24, 2023 19:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants