Optimize enums with all unit variants and empty arrays with Lazy#4237
Conversation
|
@jamie-osec is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
jamie-osec
force-pushed
the
lazy-sized-fixes
branch
2 times, most recently
from
25217ef to
25e59c2
Compare
jamie-osec
force-pushed
the
lazy-sized-fixes
branch
from
25e59c2 to
c2e8c53
Compare
acheroncrypto
left a comment
acheroncrypto
left a comment
There was a problem hiding this comment.
It would be better to have separate PRs for optimizations and generics impl since they are mostly unrelated and don't depend on each other. Among many other benefits, this would allow clear and concise PR titles such as
- Optimize enums with all unit variants in
Lazy - Add generics support to
Lazy(to me, this is more of a feature than a fix because the impl wasn't broken; it simply didn't support generics)
instead of "Collection of fixes to Lazy", which is quite vague.
| // TODO: This method can be optimized to *only* serialize the fields that we have | ||
| // initialized rather than deserializing the whole account, and then serializing it | ||
| // back, which consumes a lot more CUs than it should for most accounts. | ||
| fn exit(&self, program_id: &anchor_lang::prelude::Pubkey) -> anchor_lang::Result<()> { |
There was a problem hiding this comment.
Just FYI since you're already at it: by far the most inefficient part of lazy accounts is this part. It's certainly possible to do all sorts of optimizations here in case you're interested.
jamie-osec
force-pushed
the
lazy-sized-fixes
branch
2 times, most recently
from
489d279 to
698c13a
Compare
jamie-osec
changed the title
LazyLazy
jamie-osec
force-pushed
the
lazy-sized-fixes
branch
from
698c13a to
58027d9
Compare
|
Kept the two |
jamie-osec
changed the title
LazyLazy
acheroncrypto
left a comment
acheroncrypto
left a comment
There was a problem hiding this comment.
Thanks for updating! It's so much easier and faster to review and merge this way.
jamie-osec
force-pushed
the
lazy-sized-fixes
branch
from
58027d9 to
25dc6e8
Compare
acheroncrypto
left a comment
acheroncrypto
left a comment
There was a problem hiding this comment.
Nice quick win!
As a side note, I want to mention that the enum optimization is a special case (unit variant only case) of a more general potential optimization: "if all variants have the same size, the enum is sized". It would make the impl a bit more complicated, which I think was the reason I didn't do it initially, but there's essentially no downside from the user PoV.
Is this determinable in a const context? We need to provide a Either way, I think I'd like to land this with the current changes, and revisit optimisations in a followup (especially any that could potentially conflict/benefit from #4240) |
I believe it's technically possible, but I'm not sure if it's worth implementing just for this.
Yeah, agreed. All good on my end. |
2 participants
Best reviewed commit-by-commit. Fixes some generics-related issues in the lazy derive, as well as making the
SIZEDcounter more accurate to allow a few more fast-paths.Finally, enables many lang features for unit/doc/integration tests in CI.