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

Make GCMemoryInfoData blittable #58738

Merged
merged 4 commits into from
Sep 18, 2021
Merged

Conversation

MichalStrehovsky
Copy link
Member

The runtime side makes assumptions about the layout of this class, but since the class is not blittable, the runtime is free to reorder the fields however it sees fit (StructLayout(LayoutKind.Sequential) only affects the managed layout of blittable classes - it's not documented to do anything to the managed layout of non-blittable classes).

This prevents improvements to managed layout such as #51416.

The runtime side makes assumptions about the layout of this class, but since the class is not blittable, the runtime is free to reorder the fields however it sees fit (`StructLayout(LayoutKind.Sequential)` only affects the *managed* layout of blittable classes - it's not documented to do anything to the managed layout of non-blittable classes).

This prevents improvements to managed layout such as dotnet#51416.
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

@MichalStrehovsky MichalStrehovsky changed the title Make GCMemoryInfo blittable Make GCMemoryInfoData blittable Sep 7, 2021
@MichalStrehovsky
Copy link
Member Author

      Assert failure(PID 4696 [0x00001258], Thread: 764 [0x02fc]): Consistency check failed: Managed object size does not match unmanaged object size
      man: 0x114, unman: 0x118, Name: System.GCMemoryInfoData
      FAILED: size == expectedsize

Sigh. If I'm reading the tea leaves right, his is failing on 32bit platforms because the native side will try to align the beginning of GCMemoryInfoData so that it's 8-byte aligned (adding padding after the MethodTable pointer), but the managed side doesn't do that.

I wonder if the best course of action would be to just rewrite the FCall to take a reference to the first field of GCMemoryInfoData.

@jkotas
Copy link
Member

jkotas commented Sep 18, 2021

@MichalStrehovsky PTLA at my suggested fix.

@MichalStrehovsky
Copy link
Member Author

@MichalStrehovsky PTLA at my suggested fix.

Clever! Thank you!

I've re-kicked the failing CI leg because it looks like the known infra issue (collection was modified).

@jkotas jkotas merged commit a842e7a into dotnet:main Sep 18, 2021
@MichalStrehovsky MichalStrehovsky deleted the blittableGcMeminfo branch September 18, 2021 06:08
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
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.

2 participants