Only create MethodTable optional fields when needed (#79435)

Noticed a difference between statically built and dynamically built `MethodTable` for `int*[]`.
dotnet · Dec 10, 2022 · e31ddfd · e31ddfd
1 parent 9b5b977
commit e31ddfd
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 16 deletions.
diff --git a/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs b/src/coreclr/nativeaot/Common/src/Internal/Runtime/MethodTable.cs
@@ -858,10 +858,13 @@ internal bool HasDispatchMap
                 if (NumInterfaces == 0)
                     return false;
                 byte* optionalFields = OptionalFieldsPtr;
-                if (optionalFields == null)
-                    return false;
-                uint idxDispatchMap = OptionalFieldsReader.GetInlineField(optionalFields, EETypeOptionalFieldTag.DispatchMap, 0xffffffff);
-                if (idxDispatchMap == 0xffffffff)
+
+                const uint NoDispatchMap = 0xffffffff;
+                uint idxDispatchMap = NoDispatchMap;
+                if (optionalFields != null)
+                    idxDispatchMap = OptionalFieldsReader.GetInlineField(optionalFields, EETypeOptionalFieldTag.DispatchMap, NoDispatchMap);
+
+                if (idxDispatchMap == NoDispatchMap)
                 {
                     if (IsDynamicType)
                         return DynamicTemplateType->HasDispatchMap;
@@ -878,12 +881,15 @@ internal DispatchMap* DispatchMap
                 if (NumInterfaces == 0)
                     return null;
                 byte* optionalFields = OptionalFieldsPtr;
-                if (optionalFields == null)
-                    return null;
-                uint idxDispatchMap = OptionalFieldsReader.GetInlineField(optionalFields, EETypeOptionalFieldTag.DispatchMap, 0xffffffff);
-                if (idxDispatchMap == 0xffffffff && IsDynamicType)
+                const uint NoDispatchMap = 0xffffffff;
+                uint idxDispatchMap = NoDispatchMap;
+                if (optionalFields != null)
+                    idxDispatchMap = OptionalFieldsReader.GetInlineField(optionalFields, EETypeOptionalFieldTag.DispatchMap, NoDispatchMap);
+                if (idxDispatchMap == NoDispatchMap)
                 {
-                    return DynamicTemplateType->DispatchMap;
+                    if (IsDynamicType)
+                        return DynamicTemplateType->DispatchMap;
+                    return null;
                 }
 
                 if (SupportsRelativePointers)

diff --git a/...eclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs b/...eclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/EETypeCreator.cs
@@ -217,14 +217,18 @@ private static void CreateEETypeWorker(MethodTable* pTemplateEEType, uint hashCo
                 if (state.ThreadDataSize != 0)
                     rareFlags |= (uint)EETypeRareFlags.IsDynamicTypeWithThreadStatics;
 
-                optionalFields.SetFieldValue(EETypeOptionalFieldTag.RareFlags, rareFlags);
+                if (rareFlags != 0)
+                    optionalFields.SetFieldValue(EETypeOptionalFieldTag.RareFlags, rareFlags);
 
                 // Dispatch map is fetched from template type
                 optionalFields.ClearField(EETypeOptionalFieldTag.DispatchMap);
 
                 // Compute size of optional fields encoding
                 cbOptionalFieldsSize = optionalFields.Encode();
-                Debug.Assert(cbOptionalFieldsSize > 0);
+
+                // Clear the optional fields flag. We'll set it if we set optional fields later in this method.
+                if (cbOptionalFieldsSize == 0)
+                    flags &= ~(uint)EETypeFlags.OptionalFieldsFlag;
 
                 // Note: The number of vtable slots on the MethodTable to create is not necessary equal to the number of
                 // vtable slots on the template type for universal generics (see ComputeVTableLayout)
@@ -243,7 +247,7 @@ private static void CreateEETypeWorker(MethodTable* pTemplateEEType, uint hashCo
                         numVtableSlots,
                         runtimeInterfacesLength,
                         hasFinalizer,
-                        true,
+                        cbOptionalFieldsSize > 0,
                         (rareFlags & (int)EETypeRareFlags.HasSealedVTableEntriesFlag) != 0,
                         isGeneric,
                         allocatedNonGCDataSize != 0,
@@ -278,10 +282,11 @@ private static void CreateEETypeWorker(MethodTable* pTemplateEEType, uint hashCo
                     Debug.Assert(pEEType->HasGCPointers == (cbGCDesc != 0));
 
                     // Copy the encoded optional fields buffer to the newly allocated memory, and update the OptionalFields field on the MethodTable
-                    // It is important to set the optional fields first on the newly created MethodTable, because all other 'setters'
-                    // will assert that the type is dynamic, just to make sure we are not making any changes to statically compiled types
-                    pEEType->OptionalFieldsPtr = (byte*)pEEType + cbEEType;
-                    optionalFields.WriteToEEType(pEEType, cbOptionalFieldsSize);
+                    if (cbOptionalFieldsSize > 0)
+                    {
+                        pEEType->OptionalFieldsPtr = (byte*)pEEType + cbEEType;
+                        optionalFields.WriteToEEType(pEEType, cbOptionalFieldsSize);
+                    }
 
                     // Copy VTable entries from template type
                     IntPtr* pVtable = (IntPtr*)((byte*)pEEType + sizeof(MethodTable));