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

refactor(store): optimize Storage library #1194

Merged
merged 12 commits into from
Jul 28, 2023
Merged

refactor(store): optimize Storage library #1194

merged 12 commits into from
Jul 28, 2023

Conversation

dk1a
Copy link
Contributor

@dk1a dk1a commented Jul 26, 2023

Solidity MUD
write (cold, 1 word) 22107 22339
write (cold, 1 word, partial) 22136 22406
write (cold, 10 words) 199902 199795
write (warm, 1 word) 107 339
write (warm, 1 word, partial) 236 506
write (warm, 10 words) 1988 1795
load (cold, 1 word) 2109 2411
load (cold, 1 word, partial) 2126 2460
load (cold, 10 words) 19894 19911
load (warm, 1 word) 109 414
load (warm, 1 word, partial) 126 462
load (warm, 10 words) 1897 1935

surprised I got storage write (warm, 10 words) to be more efficient than solidity
but also it doesn't mean anything, this is apples to oranges:

// Note that this compares storage writes in isolation, which can be misleading,
// since MUD encoding is dynamic and separate from storage,
// but solidity encoding is hardcoded at compile-time and is part of writing to storage.
// (look for comparison of native storage vs MUD tables for a more comprehensive overview)

Not a fan of how big the relative difference in load (warm, 1 word) is but it's kinda unavoidable. Rough estimate of reasons:

  • internal function - 40 each, and we need 2 (not counting leftMask, optimizer can inline it) for the code to be readable/auditable
  • ~40 for jagged head/tail checks (which solidity can do at compile time); more if they actually exist, see (warm, 1 word, partial) for jagged head
  • ~145 for a cycle (which solidity can hardcode)

@changeset-bot
Copy link

changeset-bot bot commented Jul 26, 2023

🦋 Changeset detected

Latest commit: 11c04a4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 26 packages
Name Type
@latticexyz/store Patch
@latticexyz/cli Patch
@latticexyz/network Patch
@latticexyz/react Patch
@latticexyz/std-client Patch
@latticexyz/store-cache Patch
@latticexyz/store-sync Patch
@latticexyz/world Patch
@latticexyz/dev-tools Patch
@latticexyz/ecs-browser Patch
@latticexyz/block-logs-stream Patch
@latticexyz/common Patch
@latticexyz/config Patch
create-mud Patch
@latticexyz/gas-report Patch
@latticexyz/noise Patch
@latticexyz/phaserx Patch
@latticexyz/protocol-parser Patch
@latticexyz/recs Patch
@latticexyz/schema-type Patch
@latticexyz/services Patch
@latticexyz/solecs Patch
solhint-config-mud Patch
solhint-plugin-mud Patch
@latticexyz/std-contracts Patch
@latticexyz/utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment on lines +115 to +118
// Solidity's YulUtilFunctions::roundUpFunction
function round_up_to_mul_of_32(value) -> _result {
_result := and(add(value, 31), not(31))
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can't really make a solidity function for this so figured I'd just copy the one solidity uses internally

@dk1a dk1a changed the title WIP Storage gas optimizations refactor(store): optimize Storage library Jul 27, 2023
@dk1a dk1a marked this pull request as ready for review July 27, 2023 16:57
@dk1a dk1a requested review from alvrs and holic as code owners July 27, 2023 16:57
@alvrs
Copy link
Member

alvrs commented Jul 28, 2023

do we have a measurement for "load cold"?

@dk1a
Copy link
Contributor Author

dk1a commented Jul 28, 2023

do we have a measurement for "load cold"?

no, that's intentional, the cold margin is similar for read/write and didn't feel necessary to overdo gas metrics; I can add it if u're interested

alvrs
alvrs previously approved these changes Jul 28, 2023
Copy link
Member

@alvrs alvrs left a comment

Choose a reason for hiding this comment

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

this is great!

@alvrs
Copy link
Member

alvrs commented Jul 28, 2023

no, that's intentional, the cold margin is similar for read/write and didn't feel necessary to overdo gas metrics; I can add it if u're interested

just feels like something that would be good for completeness, i'm sure people will ask about it

@dk1a
Copy link
Contributor Author

dk1a commented Jul 28, 2023

just feels like something that would be good for completeness, i'm sure people will ask about it

sure will add in a few mins; we shouldn't make these metrics too visible for end users though, that'd be the role of table metrics (which I have yet to add). This stuff is kinda misleading and concerns an internal library most people don't need to understand

@dk1a dk1a deleted the dk1a/storage-gas branch July 28, 2023 14:28
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