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

Remove the concept of packable fields from EEClass #96699

Conversation

davidwrighton
Copy link
Member

  • EEClass has long had a concept of packable rarely accessed fields
  • When we had NGEN we had logic to make these fields take very small amounts of space, but now that we no longer have NGEN, this code is not actually used in practice
  • The unpacked form of these fields used 4 bytes per field, even though many of them were logically 2 byte fields
  • The NumBoxed fields were just unnecessary, especially if we have a HasBoxedThreadStatics flag, which this change adds
  • Move the HasBoxed flags to the WFLAGS_LOW_ENUM

Overall this saves 24 bytes per EEClass and moves all these fields onto a normal structure.

- EEClass has long had a concept of packable rarely accessed fields
- When we had NGEN we had logic to make these fields take very small amounts of space, but now that we no longer have NGEN, this code is not actually used in practice
- The unpacked form of these fields used 4 bytes per field, even though many of them were logically 2 byte fields
- The NumBoxed fields were just unnecessary, especially if we have a HasBoxedThreadStatics flag, which this change adds
- Move the HasBoxed flags to the WFLAGS_LOW_ENUM

Overall this saves 24 bytes per EEClass and moves all these fields onto a normal structure.
src/coreclr/vm/array.cpp Outdated Show resolved Hide resolved
davidwrighton and others added 4 commits January 9, 2024 12:52
Co-authored-by: Austin Wise <AustinWise@gmail.com>
….com:davidwrighton/runtime into remove_special_ngen_thing_for_EEClass_fields
@@ -209,7 +190,7 @@ EEClass::CreateMinimalClass(LoaderHeap *pHeap, AllocMemTracker *pamTracker)
}
CONTRACTL_END;

return new (pHeap, pamTracker) EEClass(sizeof(EEClass));
return new (pHeap, pamTracker) EEClass();
Copy link
Member

Choose a reason for hiding this comment

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

Does CreateMinimalClass still make sense as a separate helper now that 'minimal' is just normal?

Copy link
Member Author

Choose a reason for hiding this comment

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

We still want to allocate using the LoaderHeap as that is what controls object lifetime. We have no mechanism in place to run delete and free memory for these piecemeal.

@davidwrighton davidwrighton merged commit 8b581ca into dotnet:main Jan 24, 2024
111 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants