Place frozen objects into dehydrated section #78688

MichalStrehovsky · 2022-11-22T08:23:15Z

Saves 1% in size on a Hello World on Windows, more on Linux because we're getting rid of full pointers.

Instead of using ArrayOfEmbeddedDataNode as the base for the frozen object region, derive from DehydratableObjectNode directly. The base class wasn't really saving us much work over ObjectNode anyway.
Move the "align object to minimum size" code to the central location while I'm touching it.
Keep track of all frozen objects in the MetadataManager. This is the general pattern we follow elsewhere. The old pattern where the frozen object adds itself from the callback also works, but this matches the approach we use for other summarized node kinds.

Cc @dotnet/ilc-contrib

Saves 1% in size on a Hello World on Windows, more on Linux because we're getting rid of full pointers. * Instead of using `ArrayOfEmbeddedDataNode` as the base for the frozen object region, derive from `DehydratableObjectNode` directly. The base class wasn't really saving us much work over `ObjectNode` anyway. * Move the "align object to minimum size" code to the central location while I'm touching it. * Keep track of all frozen objects in the `MetadataManager`. This is the general pattern we follow elsewhere. The old pattern where the frozen object adds itself from the callback also works, but this matches the approach we use for other summarized node kinds.

ghost · 2022-11-22T08:23:33Z

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

Issue Details

Saves 1% in size on a Hello World on Windows, more on Linux because we're getting rid of full pointers.

Instead of using ArrayOfEmbeddedDataNode as the base for the frozen object region, derive from DehydratableObjectNode directly. The base class wasn't really saving us much work over ObjectNode anyway.
Move the "align object to minimum size" code to the central location while I'm touching it.
Keep track of all frozen objects in the MetadataManager. This is the general pattern we follow elsewhere. The old pattern where the frozen object adds itself from the callback also works, but this matches the approach we use for other summarized node kinds.

Cc @dotnet/ilc-contrib

Author:	MichalStrehovsky
Assignees:	-
Labels:	`area-NativeAOT-coreclr`
Milestone:	-

agocke · 2022-11-22T18:24:50Z

...oreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ArrayOfFrozenObjectsNode.cs

 using Internal.TypeSystem;

 namespace ILCompiler.DependencyAnalysis
 {
-    public class ArrayOfFrozenObjectsNode<TEmbedded> : ArrayOfEmbeddedDataNode<TEmbedded>
-        where TEmbedded : EmbeddedObjectNode
+    public class ArrayOfFrozenObjectsNode : DehydratableObjectNode, ISymbolDefinitionNode


Dehydratable I assume means serializable. This is an interesting design. What happens if there are cycles in this graph? Are they correctly preserved?

Dehydratable means we will run the code from #77884 on this and "dehydrate" pointers and excess zeros in the data blob this node generated into a more succinct representation in the executable image. It will get "rehydrated" at startup.

For cycles in the graph of frozen objects: those can't happen - we don't allow freezing objects with mutable reference-typed fields. The write barriers don't handle frozen segment and the GC wouldn't handle references from those either. The freezing/serialization algorithm wouldn't probably have problems with it.

Not having writeable fields does not prevent cycles. The following will have a cycle:

class c1 { private readonly c1 Self; c1() => Self = this; }

Can we have this in frozen objects?

(I am mostly curious if there are other restrictions that will prevent cycles. I do not think dehydration will have problems)

That one bails at the stfld:

runtime/src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs

Lines 605 to 608 in 1076cf5

if (field.FieldType.IsGCPointer && value != null)

{

return Status.Fail(methodIL.OwningMethod, opcode, "Reference field");

}

In theory the cycle should be fine because the reference would be just a reloc to another symbol and the data blob associated with that symbol has another reloc.

I think the only reference-typed instance fields we currently really allow are those of delegates to instance methods, and those are pretty special.

agocke · 2022-11-22T18:26:12Z

...oreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ArrayOfFrozenObjectsNode.cs

    {
-        public ArrayOfFrozenObjectsNode(string startSymbolMangledName, string endSymbolMangledName, IComparer<TEmbedded> nodeSorter) : base(startSymbolMangledName, endSymbolMangledName, nodeSorter)
+        private readonly ObjectAndOffsetSymbolNode _endSymbol;


Does this get dehyrated as a pointer? Or written inline?

If the former, is there a big benefit to the extra pointer?

This defines an additional symbol - it's up to whoever references it how they'll do it. ReadyToRun header currently references it as a full pointer. It doesn't have to. It's one of those things I'm planning to convert to 32bit relative relocs (although this one doesn't contribute much).

agocke · 2022-11-22T18:28:40Z

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs

@@ -1158,7 +1155,7 @@ public virtual void AttachToDependencyGraph(DependencyAnalyzerBase<NodeFactory>
            ReadyToRunHeader.Add(ReadyToRunSectionType.EagerCctor, EagerCctorTable, EagerCctorTable.StartSymbol, EagerCctorTable.EndSymbol);
            ReadyToRunHeader.Add(ReadyToRunSectionType.TypeManagerIndirection, TypeManagerIndirection, TypeManagerIndirection);
            ReadyToRunHeader.Add(ReadyToRunSectionType.InterfaceDispatchTable, DispatchMapTable, DispatchMapTable.StartSymbol);
-            ReadyToRunHeader.Add(ReadyToRunSectionType.FrozenObjectRegion, FrozenSegmentRegion, FrozenSegmentRegion.StartSymbol, FrozenSegmentRegion.EndSymbol);
+            ReadyToRunHeader.Add(ReadyToRunSectionType.FrozenObjectRegion, FrozenSegmentRegion, FrozenSegmentRegion, FrozenSegmentRegion.EndSymbol);


What's a "ReadyToRunHeader"? Is this documented anywhere?

It's a data structure similar to the ReadyToRun header in R2R images. There might be a plan to unify aspects of this format at some point. The things pointed from the header are currently pretty disjoint.

The structure of the header is doc'd at https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/readytorun-format.md#readytorun_header. The main difference starts at READYTORUN_SECTION where the section type IDs are different for NativeAOT. Also the way to get to the header from "outside the image" doesn't exist. The address is inlined in code and not pointed by PE headers.

...oreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ArrayOfFrozenObjectsNode.cs

sbomer · 2022-11-29T17:31:59Z

...oreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ArrayOfFrozenObjectsNode.cs

        public override int ClassCode => -1771336339;
+
+        public override bool IsShareable => false;


I probably missed it, but I couldn't find any references to this. What does it do?

We have a (mostly test-only) mode where each managed assembly can be compiled into a separate object file, and then linked into an EXE by the linker.

In this mode, some nodes can end up being duplicated (e.g. List<int>.Add) because we don't know if any other object file could be defining that (the compilation is done in isolation). We mark such nodes as shareable. Object writing will place them into a COMDAT section and that allows linker to pick any definition (it would error out otherwise).

MichalStrehovsky requested review from sbomer and LakshanF November 22, 2022 08:23

dotnet-issue-labeler bot added the area-NativeAOT-coreclr label Nov 22, 2022

ghost assigned MichalStrehovsky Nov 22, 2022

MichalStrehovsky mentioned this pull request Nov 22, 2022

Investigate size of .rela.dyn section on Linux #76657

Closed

build-analysis bot mentioned this pull request Nov 22, 2022

Tracking issue for CI build timeouts #76454

Closed

runfoapp bot mentioned this pull request Nov 22, 2022

Compiler crashes when failing to release Mutex #53420

Open

agocke reviewed Nov 22, 2022

View reviewed changes

build-analysis bot mentioned this pull request Nov 22, 2022

FileNameStar_NeedsEncoding_EncodedAndDecodedCorrectly test is failing in CI #78718

Closed

MichalStrehovsky mentioned this pull request Nov 23, 2022

Place dispatch cells in dehydrated section #78748

Merged

sbomer approved these changes Nov 29, 2022

View reviewed changes

VSadov approved these changes Nov 29, 2022

View reviewed changes

agocke mentioned this pull request Nov 30, 2022

Native AOT Trimming & Perf Improvements in .NET 8 #79003

Closed

14 tasks

Update ArrayOfFrozenObjectsNode.cs

3866699

MichalStrehovsky merged commit 849e9b3 into dotnet:main Nov 30, 2022

MichalStrehovsky deleted the frozenDehydrate branch November 30, 2022 05:49

ghost locked as resolved and limited conversation to collaborators Dec 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Place frozen objects into dehydrated section #78688

Place frozen objects into dehydrated section #78688

MichalStrehovsky commented Nov 22, 2022

ghost commented Nov 22, 2022

agocke Nov 22, 2022

MichalStrehovsky Nov 22, 2022

VSadov Nov 29, 2022

MichalStrehovsky Nov 30, 2022

agocke Nov 22, 2022

MichalStrehovsky Nov 22, 2022

agocke Nov 22, 2022

MichalStrehovsky Nov 22, 2022

MichalStrehovsky Nov 22, 2022

sbomer Nov 29, 2022

MichalStrehovsky Nov 30, 2022

	if (field.FieldType.IsGCPointer && value != null)
	{
	return Status.Fail(methodIL.OwningMethod, opcode, "Reference field");
	}

		public override int ClassCode => -1771336339;

		public override bool IsShareable => false;

Place frozen objects into dehydrated section #78688

Place frozen objects into dehydrated section #78688

Conversation

MichalStrehovsky commented Nov 22, 2022

ghost commented Nov 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment