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

Optimize Storage read/write api #4795

Merged
merged 19 commits into from Sep 14, 2023
Merged

Optimize Storage read/write api #4795

merged 19 commits into from Sep 14, 2023

Conversation

bitzoic
Copy link
Member

@bitzoic bitzoic commented Jul 12, 2023

Description

The storage api currently reads in all storage from 0 to offset. This has been optimized to only read the required storage slots.

Closes #4789

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@bitzoic bitzoic added the lib: std Standard library label Jul 12, 2023
@bitzoic bitzoic self-assigned this Jul 12, 2023
@bitzoic bitzoic linked an issue Jul 12, 2023 that may be closed by this pull request
@bitzoic bitzoic marked this pull request as ready for review September 11, 2023 14:17
@bitzoic bitzoic requested a review from a team September 11, 2023 14:17
sway-lib-std/src/storage/storage_api.sw Outdated Show resolved Hide resolved
sway-lib-std/src/storage/storage_api.sw Outdated Show resolved Hide resolved
@bitzoic bitzoic requested review from Braqzen and a team September 11, 2023 16:09
@bitzoic bitzoic added the breaking May cause existing user code to break. Requires a minor or major release. label Sep 11, 2023
K1-R1
K1-R1 previously approved these changes Sep 12, 2023
Copy link
Contributor

@K1-R1 K1-R1 left a comment

Choose a reason for hiding this comment

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

LGTM. Just out of interest do you have any idea on the performance benefits?

sway-lib-std/src/storage/storage_api.sw Outdated Show resolved Hide resolved
@bitzoic
Copy link
Member Author

bitzoic commented Sep 12, 2023

LGTM. Just out of interest do you have any idea on the performance benefits?

The performance is negligible on small structures or values under the size of a storage slot. The performance gain is really mostly seen in things that store many variables at the same key with offsets such as the StorageVec and things like lists.

It effectively turns reading and writing from an O(n) function to O(1) where only the minimal viable storage is read/written.

@bitzoic bitzoic requested a review from a team September 13, 2023 06:24
@bitzoic bitzoic requested a review from K1-R1 September 13, 2023 06:24
sway-lib-std/src/storage/storage_api.sw Show resolved Hide resolved
sway-lib-std/src/storage/storage_api.sw Show resolved Hide resolved
sway-lib-std/src/storage/storage_api.sw Outdated Show resolved Hide resolved
sway-lib-std/src/storage/storage_api.sw Show resolved Hide resolved
@bitzoic bitzoic requested review from Braqzen and a team September 13, 2023 08:17
@bitzoic bitzoic requested a review from a team September 14, 2023 07:48
@bitzoic bitzoic merged commit 1ba013a into master Sep 14, 2023
32 checks passed
@bitzoic bitzoic deleted the bitzoic-4789 branch September 14, 2023 09:25
JoshuaBatty pushed a commit that referenced this pull request Sep 15, 2023
## Description

The storage api currently reads in all storage from `0` to `offset`.
This has been optimized to only read the required storage slots.

Closes #4789

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: bitzoic <bitzoic.eth@gmail.com>
Co-authored-by: SwayStar123 <46050679+SwayStar123@users.noreply.github.com>
bitzoic added a commit that referenced this pull request Sep 26, 2023
## Description

Currently, `StorageVec` and `Vec` do not have any relation with one
another. This PR takes advantage of the recent optimization and
refactoring done to the Storage API `read()` and `write()` functions in
#4795 and enables the storing of a
`Vec` using the `StorageVec` type.

To do this, the `StorageVec` now stores things sequentially rather than
a different key for every element. Due to the optimizations done to the
Storage API, this has become feasible as we can now load a single
element of the sequential `StorageVec`.

The storing of elements sequentially mimics the existing `Vec`, allowing
us to store it as a `raw_slice` with a ***single*** read/write to
storage as opposed to looping over the `Vec` and having ***n***
read/writes.

It should be noted that due to
#409, the storing of a `Vec` is
written in the `StorageVec` file. This is the resulting syntax:
```sway
let my_vec = Vec::new();
storage.storage_vec.store_vec(my_vec); // Store the Vec
let other_vec = storage.storage_vec.load_vec(); // Read the Vec
```
When this issue is resolved, this should be changed to a `From`
implementation changing the syntax to:
```sway
let my_vec = Vec::new();
storage.storage_vec = my_vec.into(); // Store the Vec
let other_vec = Vec::from(storage.storage_vec); // Read the Vec
```

Closes #2439

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: bitzoic <bitzoic.eth@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking May cause existing user code to break. Requires a minor or major release. lib: std Standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize storage read/write api
4 participants