Skip to content

Mark UInt128 and other obvious readonly structs as readonly. #1496

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

Merged
merged 1 commit into from
Nov 2, 2018

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Nov 1, 2018

This is a follow-up to #1475. Using the in keyword with structs that aren't marked as readonly can cause perf issues due to hidden copies being made.

UInt128 is used in ValueMapper and potentially other places that may use the in keyword. Marking it as readonly, and while I was here - going through all struct declarations and marking the obvious ones as readonly as well.

Copy link
Contributor

@TomFinley TomFinley left a comment

Choose a reason for hiding this comment

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

Ha, nice thanks @eerhardt.... Though in principle we've generally tried to have our structs be immutable so once this goes in I'm probably going to have a peek at whatever is left. :-)

@eerhardt
Copy link
Member Author

eerhardt commented Nov 1, 2018

There's a few of them left. The biggest buckets are:

And there are other cases, but more on a 'case-by-case' basis.

@eerhardt eerhardt merged commit 850f91c into dotnet:master Nov 2, 2018
@eerhardt eerhardt deleted the ReadOnlyStructs branch November 2, 2018 18:55
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants