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

[Perf] Use heuristics to avoid allocations in Sanitizer::str_till_eol #2517

Closed
wants to merge 5 commits into from

Conversation

ljedrz
Copy link
Collaborator

@ljedrz ljedrz commented Jul 10, 2024

(transferred from ProvableHQ#6, and asking @d0cd for a review as suggested there)

The Sanitizer is used very prominently in our parsing functions, and it is also a source of many allocations, most of which are temporary and avoidable.

The potential perf improvements are quite large, and I've measured them both with a 15-minute run of a --dev node and using hyperfine on a small binary that parsed all the valid .aleo programs currently present in the snarkVM codebase.

dev node:

  • all allocs are down ~15%, of which almost all are temporary
  • in Program::from_str specifically, allocs are reduced by ~64%, of which temp allocs ~88%

parsing all .aleo programs using Program::from_str:

  • allocs are reduced by ~41%
  • growth reallocs are down ~70%
  • runtime is reduced by ~31%

@ljedrz ljedrz requested a review from d0cd July 10, 2024 08:52
@ljedrz ljedrz changed the title [perf] Use heuristics to avoid allocations in Sanitizer::str_till_eol [Perf] Use heuristics to avoid allocations in Sanitizer::str_till_eol Jul 10, 2024
@ljedrz
Copy link
Collaborator Author

ljedrz commented Jul 10, 2024

CI run link

@raychu86
Copy link
Contributor

@ljedrz Could you fix the cargo fmt issue and rerun CI?

@ljedrz
Copy link
Collaborator Author

ljedrz commented Sep 25, 2024

The fmt issue doesn't come from this PR, but seems to be caused by the new nightly compiler, meaning it would happen in all the other PRs as well. While I recommend it to be handled in its own PR, this is a new CI branch for this one.

@vicsn
Copy link
Contributor

vicsn commented Oct 22, 2024

Superseded by #2563 to merge in aleonet/staging

Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
@ljedrz ljedrz force-pushed the perf/parsing_sanitizer2 branch from 831cdf6 to 42d999b Compare November 5, 2024 10:57
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
Signed-off-by: ljedrz <ljedrz@users.noreply.github.com>
@ljedrz
Copy link
Collaborator Author

ljedrz commented Nov 13, 2024

Superseded by #2563.

@ljedrz ljedrz closed this Nov 13, 2024
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