Skip to content

Place non-GC statics in the dehydratable section #78795

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.

Sign up for GitHub

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

Merged
MichalStrehovsky merged 1 commit into from
Nov 30, 2022
Merged

Place non-GC statics in the dehydratable section #78795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNodeSection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public bool IsStandardSection
public static readonly ObjectNodeSection FoldableReadOnlyDataSection = new ObjectNodeSection("rdata", SectionType.ReadOnly);
public static readonly ObjectNodeSection TextSection = new ObjectNodeSection("text", SectionType.Executable);
public static readonly ObjectNodeSection TLSSection = new ObjectNodeSection("TLS", SectionType.Writeable);
public static readonly ObjectNodeSection BssSection = new ObjectNodeSection("bss", SectionType.Writeable);
public static readonly ObjectNodeSection BssSection = new ObjectNodeSection("bss", SectionType.Uninitialized);
Copy link
Member Author

@MichalStrehovsky MichalStrehovsky Nov 24, 2022

Choose a reason for hiding this comment

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

I've confirmed in the CI logs this also fixes the linker warning in multimodule testing: MultiModule.obj : warning LNK4078: multiple '.bss' sections found with different attributes (C0401040). Yay.

public static readonly ObjectNodeSection HydrationTargetSection = new ObjectNodeSection("hydrated", SectionType.Uninitialized);
public static readonly ObjectNodeSection ManagedCodeWindowsContentSection = new ObjectNodeSection(".managedcode$I", SectionType.Executable);
public static readonly ObjectNodeSection FoldableManagedCodeWindowsContentSection = new ObjectNodeSection(".managedcode$I", SectionType.Executable);
Expand Down
20 changes: 16 additions & 4 deletions ...reclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DehydratableObjectNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,28 @@ public abstract class DehydratableObjectNode : ObjectNode
{
public sealed override ObjectNodeSection GetSection(NodeFactory factory)
{
return factory.MetadataManager.IsDataDehydrated ? ObjectNodeSection.HydrationTargetSection : GetDehydratedSection(factory);
ObjectNodeSection desiredSection = GetDehydratedSection(factory);

return factory.MetadataManager.IsDataDehydrated
&& desiredSection.Type != SectionType.Uninitialized
? ObjectNodeSection.HydrationTargetSection : desiredSection;
}

public sealed override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
{
ObjectData result = GetDehydratableData(factory, relocsOnly);

// If we're not generating full data yet, or dehydration is not active,
// return the ObjectData as is.
if (relocsOnly || !factory.MetadataManager.IsDataDehydrated)
// If we're not actually generating data yet, don't dehydrate
bool skipDehydrating = relocsOnly;

// If dehydration is not active, don't dehydrate
skipDehydrating |= !factory.MetadataManager.IsDataDehydrated;

// If the data would be placed into an uninitialized section, that's better
// than dehydrating a bunch of zeros.
skipDehydrating |= GetDehydratedSection(factory).Type == SectionType.Uninitialized;
Copy link
Member

@sbomer sbomer Nov 30, 2022

Choose a reason for hiding this comment

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

nit: maybe the shared logic could be moved into a helper to ensure it's consistent between GetSection and GetData (I had to read this a couple times to convince myself it was right, but it may just be me).

Copy link
Member Author

@MichalStrehovsky MichalStrehovsky Nov 30, 2022

Choose a reason for hiding this comment

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

I tried with a bool CanDehydrate(ObjectNodeSection desiredSection) but the shape of the helper (need to pass the section to make a decision) made it a bit weird. I'll keep it like this for now.


if (skipDehydrating)
return result;

// Otherwise return the dehydrated data
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NonGCStaticsNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace ILCompiler.DependencyAnalysis
/// with the class constructor context if the type has a class constructor that
/// needs to be triggered before the type members can be accessed.
/// </summary>
public class NonGCStaticsNode : ObjectNode, ISymbolDefinitionNode, ISortableSymbolNode
public class NonGCStaticsNode : DehydratableObjectNode, ISymbolDefinitionNode, ISortableSymbolNode
{
private readonly MetadataType _type;
private readonly PreinitializationManager _preinitializationManager;
Expand All @@ -31,7 +31,7 @@ public NonGCStaticsNode(MetadataType type, PreinitializationManager preinitializ

protected override string GetName(NodeFactory factory) => this.GetMangledName(factory.NameMangler);

public override ObjectNodeSection GetSection(NodeFactory factory)
protected override ObjectNodeSection GetDehydratedSection(NodeFactory factory)
{
if (_preinitializationManager.HasLazyStaticConstructor(_type)
|| _preinitializationManager.IsPreinitialized(_type))
Expand Down Expand Up @@ -118,7 +118,7 @@ protected override DependencyList ComputeNonRelocationBasedDependencies(NodeFact
return dependencyList;
}

public override ObjectData GetData(NodeFactory factory, bool relocsOnly)
protected override ObjectData GetDehydratableData(NodeFactory factory, bool relocsOnly)
{
ObjectDataBuilder builder = new ObjectDataBuilder(factory, relocsOnly);

Expand Down