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

struct Av1Block: Replace Rust enum with separate tag and union #1331

Closed
wants to merge 1 commit into from

Conversation

rinon
Copy link
Collaborator

@rinon rinon commented Jul 19, 2024

This saves 4 bytes x number of Av1Block allocations by making Av1Block 32 bytes instead of 36. We can't just make the previous ii field a Rust enum with a 1-byte discriminant because the contents must be 2-byte aligned due to Mv needing that alignment.

On Chimera 8-bit this saves about 2% of the total heap usage.

Copy link
Collaborator

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

We can also make Av1Block 32 bytes simply by removing the #[repr(C)] on Av1BlockIntraInter. I'll open a PR for that. I'm not sure how to measure the memory usage, but I assume that will have similar/the same memory savings.

@rinon
Copy link
Collaborator Author

rinon commented Jul 22, 2024

Turns out we don't need this, I didn't get the right combination (#1333) of reprs that reduced the size to 32

@rinon rinon closed this Jul 22, 2024
kkysen added a commit that referenced this pull request Jul 22, 2024
…tes (#1333)

This is a simpler, fully safe alternative to #1331. It reduces
`Av1BlockIntraInter` from 28 to 24 bytes, and `Av1Block` from 36 to 32
bytes, just like #1331.

I haven't checked memory usage, though, since I'm not sure how to do
that, but since it reduces the type sizes the same, I assume it would
have the same memory savings.
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