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

Precompute storage offsets #1414

Closed
wants to merge 6 commits into from
Closed

Conversation

edfelten
Copy link
Contributor

@edfelten edfelten commented Jan 5, 2023

This adds new affordances for accessing mapped storage in ArbOS, so that storage offsets can be computed at compile/initialization time, rather than being recomputed each time a data structure is opened. This is useful because storage mapping relies on Keccak256 hashing, which has nontrivial cost to compute.

The basic strategy is to precompute a storage map data structure, which records the hashed offsets of data objects within a storage-backed structure. Then when we open the structure we can use the precomputed storage map, rather than recomputing the hashes.

The L1 pricer and L2 pricer structures are rewritten to use the new affordances, because they are the largest and most complex storage-backed structures in ArbOS. Other data structures are not rewritten. I'm interested in getting reviewer opinions on whether to rewrite more structures in this PR.

This does not require a new format version because the layout of data in storage doesn't change. We only change when we compute the layout.

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Jan 5, 2023
@codecov
Copy link

codecov bot commented Jan 5, 2023

Codecov Report

Merging #1414 (b64bd7f) into master (aa9adec) will decrease coverage by 3.43%.
The diff coverage is 90.16%.

@@            Coverage Diff             @@
##           master    #1414      +/-   ##
==========================================
- Coverage   52.25%   48.81%   -3.44%     
==========================================
  Files         260      248      -12     
  Lines       33906    29942    -3964     
  Branches      555      555              
==========================================
- Hits        17717    14617    -3100     
+ Misses      14125    13272     -853     
+ Partials     2064     2053      -11     

@PlasmaPower PlasmaPower requested a review from magicxyyz June 9, 2023 06:44
@joshuacolvin0
Copy link
Member

Went with #1767 instead, caching values at runtime has a slightly lower concern about possibly using incorrect offsets.

@joshuacolvin0 joshuacolvin0 deleted the precompute-storage-offsets branch September 28, 2023 03:01
@AiyAbuya AiyAbuya mentioned this pull request Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants