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

enum TileStateRef: Manual impl Atom instead of #[derive(Atom)]ing it, since the derive may panic #1247

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

kkysen
Copy link
Collaborator

@kkysen kkysen commented Jun 25, 2024

It is more optimal to .unwrap_or_default(), which is a cmov, rather than branching to a panic message, which bloats the code a lot.

This reduces stack usage in fn decode_coefs by 64 bytes, from 296 to 232.

@kkysen kkysen requested review from randomPoison and fbossen June 25, 2024 10:28
@kkysen kkysen force-pushed the kkysen/enum-CfSelect-optimize branch from a7c736a to 7065142 Compare July 1, 2024 19:06
@kkysen kkysen force-pushed the kkysen/enum-TileStateRef-manual-impl-Atom branch 2 times, most recently from e16709e to 029f56e Compare July 1, 2024 19:21
@kkysen kkysen force-pushed the kkysen/enum-CfSelect-optimize branch from 7065142 to 6155d6b Compare July 1, 2024 19:21
…ng it, since the derive may panic.

It is more optimal to `.unwrap_or_default()`, which is a `cmov`,
rather than branching to a panic message, which bloats the code a lot.

This reduces stack usage in `fn decode_coefs` by 64 bytes, from 296 to 232.
@kkysen kkysen force-pushed the kkysen/enum-CfSelect-optimize branch from 6155d6b to 810942f Compare July 1, 2024 19:24
@kkysen kkysen force-pushed the kkysen/enum-TileStateRef-manual-impl-Atom branch from 029f56e to 951d69d Compare July 1, 2024 19:24
Base automatically changed from kkysen/enum-CfSelect-optimize to main July 1, 2024 21:20
@kkysen kkysen merged commit 3f55807 into main Jul 1, 2024
26 checks passed
@kkysen kkysen deleted the kkysen/enum-TileStateRef-manual-impl-Atom branch July 1, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants