Skip to content

feat: add more lazy removal of instructions with Instruction::noop#7035

Closed
TomAFrench wants to merge 1 commit intomasterfrom
tf/replace-retain-with-noops
Closed

feat: add more lazy removal of instructions with Instruction::noop#7035
TomAFrench wants to merge 1 commit intomasterfrom
tf/replace-retain-with-noops

Conversation

@TomAFrench
Copy link
Member

Description

Problem*

Followup to #6899

Summary*

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Execution Memory Report

Program Peak Memory
keccak256 74.67M
workspace 123.69M
regression_4709 316.00M
ram_blowup_regression 512.61M
rollup-base-public 478.82M
rollup-base-private 325.41M
private-kernel-tail 180.42M
private-kernel-reset 245.11M
private-kernel-inner 208.50M

@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Compilation Memory Report

Program Peak Memory
keccak256 78.13M
workspace 123.72M
regression_4709 424.09M
ram_blowup_regression 1.58G
rollup-base-public 4.85G
rollup-base-private 1.26G
private-kernel-tail 207.14M
private-kernel-reset 669.23M
private-kernel-inner 294.37M

@TomAFrench TomAFrench marked this pull request as draft January 13, 2025 13:09
@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Compilation Report

Program Compilation Time %
sha256_regression 1.290s 4%
regression_4709 0.828s 7%
ram_blowup_regression 18.140s -6%
rollup-root 3.522s 5%
rollup-merge 2.114s 14%
rollup-block-merge 3.718s 11%
rollup-base-public 43.880s 5%
rollup-base-private 13.400s 0%
private-kernel-tail 0.972s 0%
private-kernel-reset 7.352s -3%
private-kernel-inner 2.038s 2%

@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2025

Execution Report

Program Execution Time %
sha256_regression 0.051s -2%
regression_4709 0.001s 0%
ram_blowup_regression 0.603s -1%
rollup-root 0.105s 0%
rollup-merge 0.007s 16%
rollup-block-merge 0.106s 1%
rollup-base-public 1.234s 0%
rollup-base-private 0.452s 0%
private-kernel-tail 0.019s 0%
private-kernel-reset 0.313s 0%
private-kernel-inner 0.068s 0%

@TomAFrench TomAFrench force-pushed the tf/replace-retain-with-noops branch from ae72537 to 7087c8d Compare January 13, 2025 13:45
@jfecher
Copy link
Contributor

jfecher commented Jan 13, 2025

Interesting this looks like it is either increasing compilation times or has no substantial effect

@TomAFrench
Copy link
Member Author

Yeah in hindsight the benefits from avoiding rebuilding the instructions vec at the end of these passes are probably outweighed by the costs of having a bloated input to the next pass

@TomAFrench TomAFrench closed this Jan 13, 2025
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.

2 participants