Skip to content

Use auto-implemented properties #669

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 3 commits into from
Feb 15, 2025

Conversation

Joy-less
Copy link
Contributor

@Joy-less Joy-less commented Feb 5, 2025

This eliminates three instances of variable shadowing and slightly simplifies the code.

@bitfaster
Copy link
Owner

Have you verified this produces identical IL and assembly code across .NET Framework and .NET Core?

I have seen in the past that changing to auto properties or changing field order can result in the JIT changing memory layout resulting in regressions that use a lot of time to track down.

@Joy-less
Copy link
Contributor Author

Joy-less commented Feb 6, 2025

On .NET 6.0:

Before:
image

After:
image

On .NET Standard 2.0:

Before:
image

After:
image

I used ildasm.exe on BitFaster.Caching.dll. Looks to be the same memory layout to me, just the backing fields have different names.

@coveralls
Copy link

coveralls commented Feb 13, 2025

Coverage Status

coverage: 99.168% (+0.07%) from 99.102%
when pulling 44df0a1 on Joy-less:use-auto-implemented-properties
into 55ac07e on bitfaster:main.

@bitfaster bitfaster merged commit 9519352 into bitfaster:main Feb 15, 2025
13 checks passed
@Joy-less Joy-less deleted the use-auto-implemented-properties branch February 15, 2025 02:16
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.

3 participants