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.

Sign up for GitHub

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

Jump to bottom

Review HasFlag calls in hot code paths #13308

Closed
Youssef1313 opened this issue Aug 19, 2023 · 1 comment · Fixed by #13309
Closed

Review HasFlag calls in hot code paths #13308

Youssef1313 opened this issue Aug 19, 2023 · 1 comment · Fixed by #13309
Assignees
@Youssef1313
Labels
area/performance 📈 Categorizes an issue or PR as relevant to performance difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/enhancement New feature or request triage/untriaged Indicates an issue requires triaging or verification

Comments

@Youssef1313
Copy link
Member

HasFlag was optimized in .NET Core 2.1 to not box, but apparently, it's not guaranteed that this optimization will always work. See dotnet/runtime#55455 (comment)

We should replace HasFlag calls (especially ones in hot code paths like pointers) with bitwise AND.
@Youssef1313 Youssef1313 added kind/enhancement New feature or request triage/untriaged Indicates an issue requires triaging or verification difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. area/performance 📈 Categorizes an issue or PR as relevant to performance labels Aug 19, 2023
@Youssef1313 Youssef1313 self-assigned this Aug 19, 2023
@Youssef1313 Youssef1313 mentioned this issue Aug 19, 2023
Merged
6 tasks
@Youssef1313
Copy link
Member Author

Youssef1313 commented Aug 20, 2023 •
edited
Loading

Note: It looks like dotnet/runtime#89348 fixes this for .NET 8. So the HasFlag changes in #13309 could be reverted when we move to net8.0/net9.0 a year later.

Or we could just live with the bitwise AND that is known to be performant and not rely on JIT optimizing HasFlag even when we are net8.0+.

Edit: Even for net8.0 it seems that the optimization isn't guaranteed. Let's stick with HasFlag for any hot code paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Youssef1313 Youssef1313

Labels
area/performance 📈 Categorizes an issue or PR as relevant to performance difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/enhancement New feature or request triage/untriaged Indicates an issue requires triaging or verification
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

perf: Misc performance improvements Youssef1313/uno
1 participant
@Youssef1313