[NativeAOT] Cleanup usages of LowLevelDictionary #117344

huoyaoyuan · 2025-07-06T12:25:25Z

I have measured the size impact for each commit. Opening as draft for discussion.

dotnet-policy-service · 2025-07-06T12:26:14Z

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

huoyaoyuan · 2025-07-06T12:29:34Z

...Private.CoreLib/src/System/Reflection/Runtime/CustomAttributes/RuntimeCustomAttributeData.cs


            // Handle the array case
-            if (value is IEnumerable enumerableValue && !(value is string))
+            if (value is Array arr)


This single change has biggest impact (-7.6 KB in hello world with reflection!). It causes enumerator of every collection considered as potentially boxed. By removing this, many of the enumerators can be trimmed. It also reduces the size gap between Dictionary implementations.

However, this can introduce a trap for user, that using weakly-type IEnumerbale can have increased size penalty.

Extracted to #117345 for higher confidence.

huoyaoyuan · 2025-07-06T12:32:00Z

src/coreclr/nativeaot/Common/src/System/Collections/Generic/LowLevelDictionary.cs

-    ** behavior.)
-    **
-    ===========================================================*/
-    internal class LowLevelDictionary<TKey, TValue> where TKey : IEquatable<TKey>


Dictionary<TKey, TValue> has been optimized for speed and have about twice of size impact than LowLevelDictionary. Although it can reduced by sharing generic instantiations, the rare instantiations (other than Dictionary<__Canon, __Canon>) will be pure overhead. Using Dictionary more can also amplify any size impact in the shared implementation, which we are aggressively optimizing for speed.

Are we interested to keep a "size-optimized Dictionary" implementation?

huoyaoyuan · 2025-07-06T12:35:27Z

...nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs

-
-                uint index = 0;
-                if (!s_genericFunctionPointerDictionary.TryGetValue(key, out index))
+                ref IntPtr descriptor = ref CollectionsMarshal.GetValueRefOrAddDefault(s_genericFunctionPointerDictionary, (canonFunctionPointer, instantiationArgument), out bool exists);


CollectionsMarshal.GetValueRefOrAddDefault has non-trivial size win. It's the only modification method that doesn't bring in the throwing path. Even worse, the throw helpers uses generic method for keys to reduce for success path, but also increases size impact more.

Should GetValueRefOrAddDefault be preferred whenever possible?

Should GetValueRefOrAddDefault be preferred whenever possible?

I do not think we want to make the code unreadable by preferring GetValueRefOrAddDefault whenever possible. I think it is fine to use where it reduces number of required lookups like in this case.

It reduces lookups in the majority of cases. Most usages serve as a cache and follows the TryGet-Add-return structure.

Before continuing on detailed usages, I want to confirm the right strategy first. The most optimal usage of Dictionary is about 1.5x size of LowLevelDictionary per instantiation. Should we preserve or overhaul LowLevelDictionary as size-optimized alternative for rare instantiations?

I think it depends on the numbers. I do not think we want to get to write unnatural code to get a bit of extra sharing here and there. What would the total regression look like for naturally written code?

Better sharing of code between generic instantiations is a problem for the compiler to solve. These regressions are just a tip of the iceberg.

Cost of instantiation of LowLevelDictionary is 1.6KB, including interface metadata and array enumerator of KVP etc. Lookup & resize code is 800B. A wrapper IEquatable struct costs additional 0.3KB.

Cost of instantiation of Dictionary: 2.6KB in most optimal usage, lookup & resize costs 1.2KB. The equality determination path costs 0.6KB. IDictionary is not preserved, but ICollection<KVP> is preserved, costing more for interface method table. Also keeps Equals(object) for key.
The traditional lookup method of Dictionary will cost 1.3KB, +0.4 KB of basic bookkeeping. This increases the cost of instantiation become 3.1KB, or 3.9KB if both lookup logics are used.
If enumerator, Keys and Values get preserved, they will introduce 1.1KB, 1.3KB and 0.7KB for each instantiation.

The broad interface implementation of Dictionary increases the chance for members getting preserved. The grand total can be 7KB in measured workload. Even shared Dictionary<object, object> can cost ~1KB due to the interface method tables.

The unnatural trick reduces non-foldable instantiations from 13 to 7. We may also achieve this by make hashcode implementations identical.

Better sharing of code between generic instantiations is a problem for the compiler to solve. These regressions are just a tip of the iceberg.

Submitted #117411 for that, we can see how much that helps.

huoyaoyuan · 2025-07-06T12:37:42Z

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/ModuleList.cs

        /// Map of module handles to indices within the Modules array.
        /// </summary>
-        public readonly LowLevelDictionary<TypeManagerHandle, int> HandleToModuleIndex;
+        public readonly Dictionary<IntPtr, IntPtr> HandleToModuleIndex;


Tricky part: sharing 3 instantiations (<IntPtr, int>, uint, IntPtr, IntPtr, uint) into one.

huoyaoyuan · 2025-07-06T12:40:28Z

...nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs

        private const int FatFunctionPointerOffset = 2;
 #endif

-        private struct GenericMethodDescriptorInfo : IEquatable<GenericMethodDescriptorInfo>


The overhead of using tuples instead of custom struct:

ToString implementation (pure overhead)

String.Concat (surprisingly not used in hello world, very likely to be used otherwhere)

Larger implementation of GetHashCode, involving HashCode.Combine. Will be eliminated once HashCode.Combine is used elsewhere.

The conventional wisdom is that using Tuples in low-level code is bad for trimming. Have you verified that it is a net saving to use Tuples in this change?

It's net (significant) saving to fold multiple different structs into the same tuple, and net (little) regression to replace one struct with one tuple. The most-saving option should be creating sharable tuple-like struct, however it will be a regression again if user uses the same tuple.

huoyaoyuan · 2025-07-06T13:25:57Z

It seems that local measurement only covers the reflection paths. Generic instantiation of Dictionary is really heavy, with dependencies like SZGenericArrayEnumerator<KVP> that's not foldable. This increases the interest of size-optimized dictionary.

jkotas · 2025-07-06T13:54:00Z

...nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs

                {
-                    // Capture new index value
-                    index = s_genericFunctionPointerNextIndex;
+                    descriptor = (IntPtr)NativeMemory.Alloc((uint)sizeof(GenericMethodDescriptor));


If this throws, we will end up with an entry without a value. You can fix this by checking whether descriptor is null instead of whether the entry exists.

jkotas · 2025-07-06T13:58:21Z

...nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs

-                uint index = 0;
-                if (!s_genericFunctionPointerDictionary.TryGetValue(key, out index))
+                ref IntPtr descriptor = ref CollectionsMarshal.GetValueRefOrAddDefault(s_genericFunctionPointerDictionary, (canonFunctionPointer, instantiationArgument), out bool exists);
+                if (!exists)


Suggested change

if (!exists)

// Check for null descriptor instead of `exists` to handle the situation where `NativeMemory.Alloc` below threw

// an exception during earlier attempt to this descriptor

if (descriptor == IntPtr.Zero)

jkotas · 2025-07-06T13:58:40Z

...nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/FunctionPointerOps.cs

-
-                uint index = 0;
-                if (!s_genericFunctionPointerDictionary.TryGetValue(key, out index))
+                ref IntPtr descriptor = ref CollectionsMarshal.GetValueRefOrAddDefault(s_genericFunctionPointerDictionary, (canonFunctionPointer, instantiationArgument), out bool exists);


Suggested change

ref IntPtr descriptor = ref CollectionsMarshal.GetValueRefOrAddDefault(s_genericFunctionPointerDictionary, (canonFunctionPointer, instantiationArgument), out bool exists);

ref IntPtr descriptor = ref CollectionsMarshal.GetValueRefOrAddDefault(s_genericFunctionPointerDictionary, (canonFunctionPointer, instantiationArgument), out bool _);

jkotas · 2025-07-06T14:01:04Z

...nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerServices/OpenMethodResolver.cs

-                returnValue = (IntPtr)NativeMemory.Alloc((nuint)sizeof(OpenMethodResolver));
-                *((OpenMethodResolver*)returnValue) = this;
-                s_internedResolverHash.Add(this, returnValue);
+                ref IntPtr returnValue = ref CollectionsMarshal.GetValueRefOrAddDefault(s_internedResolverHash, this, out bool exists);


dotnet-policy-service · 2025-08-07T08:26:53Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

huoyaoyuan added 10 commits July 6, 2025 17:51

Eliminate IEnumerable virtual access

822455e

Remove chunk allocation in FunctionPointerOps

7fa68a9

Use dictionary instead

140682c

Use tuple instead of custom struct

0f31183

Use Dictionary of Type

5e0195b

Share instantiations for <IntPtr, object>

6cb9bb4

Share instantiations for <(IntPtr, IntPtr), IntPtr>

6898f54

Share instantiations for <IntPtr, IntPtr>

c83f413

Migrate remaining instantiations

8bf2698

Get rid of LowLevelDictionary

2582f9a

github-actions bot added the area-NativeAOT-coreclr label Jul 6, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 6, 2025

huoyaoyuan commented Jul 6, 2025

View reviewed changes

Fix projects

e15bb22

github-actions bot mentioned this pull request Jul 6, 2025

117344 MichalStrehovsky/rt-sz#141

Closed

jkotas reviewed Jul 6, 2025

View reviewed changes

dotnet-policy-service bot closed this Aug 7, 2025

github-actions bot locked and limited conversation to collaborators Sep 6, 2025

-                if (!exists)
+                // Check for null descriptor instead of `exists` to handle the situation where `NativeMemory.Alloc` below threw
+                // an exception during earlier attempt to this descriptor
+                if (descriptor == IntPtr.Zero)

	ref IntPtr descriptor = ref CollectionsMarshal.GetValueRefOrAddDefault(s_genericFunctionPointerDictionary, (canonFunctionPointer, instantiationArgument), out bool exists);
	ref IntPtr descriptor = ref CollectionsMarshal.GetValueRefOrAddDefault(s_genericFunctionPointerDictionary, (canonFunctionPointer, instantiationArgument), out bool _);

[NativeAOT] Cleanup usages of LowLevelDictionary #117344

[NativeAOT] Cleanup usages of LowLevelDictionary #117344

Uh oh!

Conversation

huoyaoyuan commented Jul 6, 2025

Uh oh!

dotnet-policy-service bot commented Jul 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huoyaoyuan Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkotas Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkotas Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huoyaoyuan commented Jul 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dotnet-policy-service bot commented Aug 7, 2025

Uh oh!

Uh oh!

huoyaoyuan Jul 6, 2025 •

edited

Loading

jkotas Jul 6, 2025 •

edited

Loading

jkotas Jul 7, 2025 •

edited

Loading