Skip to content

Commit 28deeb4

Browse files
jkoritzinskyCopilotkotlarmilos
authored
Refactor ObjectDataInterner to support pluggable deduplicators and enable for ReadyToRun (#126112)
> [!NOTE] > This PR was generated with the help of GitHub Copilot. ## Summary Refactors `ObjectDataInterner` to accept a set of pluggable `IObjectDataDeduplicator` strategies instead of hard-coding method body deduplication logic, then extends the infrastructure to ReadyToRun. ## Changes ### Pluggable deduplicator interface - Introduced `IObjectDataDeduplicator` with a single `DeduplicatePass` method. - `ObjectDataInterner` now accepts `params IObjectDataDeduplicator[]` and delegates each convergence-loop iteration to them. - Moved `ObjectDataInterner` to `Common/Compiler/` so both NativeAOT and ReadyToRun can share it. ### Method body deduplication (NativeAOT) - Extracted all `MethodDesc`/`IMethodBodyNode`-specific logic (`CanFold`, `MethodInternKey`, `MethodInternComparer`) into a new `MethodBodyDeduplicator : IObjectDataDeduplicator` class. - Added a `virtual bool CanFold(MethodDesc)` on the base `NodeFactory` (returns `false`), overridden in `RyuJitNodeFactory` to delegate to `MethodBodyDeduplicator`. ### Copied method IL deduplication (ReadyToRun) - Added `CopiedMethodILDeduplicator : IObjectDataDeduplicator` in `ILCompiler.ReadyToRun` that deduplicates `CopiedMethodILNode` instances by comparing their raw IL bytes. - Added `ObjectDataInterner` property to the ReadyToRun `NodeFactory`, wired up with the new deduplicator. - Added `Values` property to the ReadyToRun `NodeCache<TKey, TValue>` to enable enumeration. - Removed the `#if !READYTORUN` guards around `ObjectInterner.GetDeduplicatedSymbol` calls in the common `ObjectWriter`, so deduplication now runs uniformly for both pipelines. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Milos Kotlar <kotlarmilos@gmail.com>
1 parent 8c4e839 commit 28deeb4

File tree

11 files changed

+240
-81
lines changed

11 files changed

+240
-81
lines changed

‎src/coreclr/tools/Common/Compiler/ObjectDataInterner.cs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Collections.Generic;
5+
6+
using ILCompiler.DependencyAnalysis;
7+
8+
using Debug = System.Diagnostics.Debug;
9+
10+
namespace ILCompiler
11+
{
12+
/// <summary>
13+
/// Pluggable strategy for identifying and deduplicating equivalent object data.
14+
/// </summary>
15+
public interface IObjectDataDeduplicator
16+
{
17+
/// <summary>
18+
/// Performs one deduplication pass, adding entries to <paramref name="symbolRemapping"/>.
19+
/// Called iteratively until the overall mapping converges.
20+
/// </summary>
21+
void DeduplicatePass(NodeFactory factory, Dictionary<ISymbolNode, ISymbolNode> previousSymbolRemapping, Dictionary<ISymbolNode, ISymbolNode> symbolRemapping);
22+
}
23+
24+
public sealed class ObjectDataInterner
25+
{
26+
private readonly IObjectDataDeduplicator[] _deduplicators;
27+
private Dictionary<ISymbolNode, ISymbolNode> _symbolRemapping;
28+
29+
public static ObjectDataInterner Null { get; } = new ObjectDataInterner() { _symbolRemapping = new() };
30+
31+
public ObjectDataInterner(params IObjectDataDeduplicator[] deduplicators)
32+
{
33+
_deduplicators = deduplicators;
34+
}
35+
36+
private void EnsureMap(NodeFactory factory)
37+
{
38+
Debug.Assert(factory.MarkingComplete);
39+
40+
if (_symbolRemapping != null)
41+
return;
42+
43+
Dictionary<ISymbolNode, ISymbolNode> previousSymbolRemapping;
44+
Dictionary<ISymbolNode, ISymbolNode> symbolRemapping = null;
45+
46+
do
47+
{
48+
previousSymbolRemapping = symbolRemapping;
49+
symbolRemapping = new Dictionary<ISymbolNode, ISymbolNode>((int)(1.05 * (previousSymbolRemapping?.Count ?? 0)));
50+
51+
foreach (IObjectDataDeduplicator deduplicator in _deduplicators)
52+
{
53+
deduplicator.DeduplicatePass(factory, previousSymbolRemapping, symbolRemapping);
54+
}
55+
} while (!MappingsEqual(previousSymbolRemapping, symbolRemapping));
56+
57+
_symbolRemapping = symbolRemapping;
58+
}
59+
60+
private static bool MappingsEqual(Dictionary<ISymbolNode, ISymbolNode> a, Dictionary<ISymbolNode, ISymbolNode> b)
61+
{
62+
if (a is null)
63+
return false;
64+
65+
if (a.Count != b.Count)
66+
return false;
67+
68+
foreach (KeyValuePair<ISymbolNode, ISymbolNode> kvp in a)
69+
{
70+
if (!b.TryGetValue(kvp.Key, out ISymbolNode value) || value != kvp.Value)
71+
return false;
72+
}
73+
74+
return true;
75+
}
76+
77+
public ISymbolNode GetDeduplicatedSymbol(NodeFactory factory, ISymbolNode original)
78+
{
79+
EnsureMap(factory);
80+
81+
ISymbolNode target = original;
82+
if (target is ISymbolNodeWithLinkage symbolWithLinkage)
83+
target = symbolWithLinkage.NodeForLinkage(factory);
84+
85+
return _symbolRemapping.TryGetValue(target, out ISymbolNode result) ? result : original;
86+
}
87+
}
88+
}

‎src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -393,14 +393,15 @@ public virtual void EmitObject(Stream outputFileStream, IReadOnlyCollection<Depe
393393

394394
ISymbolNode symbolNode = node as ISymbolNode;
395395

396-
#if !READYTORUN
397-
ISymbolNode deduplicatedSymbolNode = _nodeFactory.ObjectInterner.GetDeduplicatedSymbol(_nodeFactory, symbolNode);
398-
if (deduplicatedSymbolNode != symbolNode)
396+
if (symbolNode is not null)
399397
{
400-
dumper?.ReportFoldedNode(_nodeFactory, node, deduplicatedSymbolNode);
401-
continue;
398+
ISymbolNode deduplicatedSymbolNode = _nodeFactory.ObjectInterner.GetDeduplicatedSymbol(_nodeFactory, symbolNode);
399+
if (deduplicatedSymbolNode != symbolNode)
400+
{
401+
dumper?.ReportFoldedNode(_nodeFactory, node, deduplicatedSymbolNode);
402+
continue;
403+
}
402404
}
403-
#endif
404405

405406
ObjectData nodeContents = node.GetData(_nodeFactory);
406407

@@ -557,10 +558,8 @@ public virtual void EmitObject(Stream outputFileStream, IReadOnlyCollection<Depe
557558
continue;
558559
}
559560

560-
#if !READYTORUN
561561
startNode = _nodeFactory.ObjectInterner.GetDeduplicatedSymbol(_nodeFactory, startNode);
562562
endNode = _nodeFactory.ObjectInterner.GetDeduplicatedSymbol(_nodeFactory, endNode);
563-
#endif
564563
Utf8String startNodeName = GetMangledName(startNode);
565564
Utf8String endNodeName = GetMangledName(endNode);
566565

@@ -579,11 +578,7 @@ public virtual void EmitObject(Stream outputFileStream, IReadOnlyCollection<Depe
579578
ArrayBuilder<Relocation> checksumRelocationsBuilder = default;
580579
foreach (Relocation reloc in blockToRelocate.Relocations)
581580
{
582-
#if READYTORUN
583-
ISymbolNode relocTarget = reloc.Target;
584-
#else
585581
ISymbolNode relocTarget = _nodeFactory.ObjectInterner.GetDeduplicatedSymbol(_nodeFactory, reloc.Target);
586-
#endif
587582

588583
if (reloc.RelocType == RelocType.IMAGE_REL_FILE_CHECKSUM_CALLBACK)
589584
{

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ internal ObjectDataInterner ObjectInterner
134134
get;
135135
}
136136

137+
protected virtual bool CanFold(MethodDesc method) => false;
138+
137139
public TypeMapManager TypeMapManager
138140
{
139141
get;
@@ -1138,7 +1140,7 @@ public IMethodNode FatFunctionPointer(MethodDesc method, bool isUnboxingStub = f
11381140

11391141
public IMethodNode FatAddressTakenFunctionPointer(MethodDesc method, bool isUnboxingStub = false)
11401142
{
1141-
if (!ObjectInterner.CanFold(method))
1143+
if (!CanFold(method))
11421144
return FatFunctionPointer(method, isUnboxingStub);
11431145

11441146
return _fatAddressTakenFunctionPointers.GetOrAdd(new MethodKey(method, isUnboxingStub));
@@ -1204,7 +1206,7 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type)
12041206
private NodeCache<MethodDesc, AddressTakenMethodNode> _addressTakenMethods;
12051207
public IMethodNode AddressTakenMethodEntrypoint(MethodDesc method, bool unboxingStub = false)
12061208
{
1207-
if (unboxingStub || !ObjectInterner.CanFold(method))
1209+
if (unboxingStub || !CanFold(method))
12081210
return MethodEntrypoint(method, unboxingStub);
12091211

12101212
return _addressTakenMethods.GetOrAdd(method);

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/ObjectDataInterner.cs renamed to src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/MethodBodyDeduplicator.cs

Lines changed: 26 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -9,27 +9,20 @@
99
using Internal.IL.Stubs;
1010
using Internal.TypeSystem;
1111

12-
using Debug = System.Diagnostics.Debug;
13-
1412
namespace ILCompiler
1513
{
16-
public sealed class ObjectDataInterner
14+
public sealed class MethodBodyDeduplicator : IObjectDataDeduplicator
1715
{
1816
private readonly bool _genericsOnly;
19-
private Dictionary<ISymbolNode, ISymbolNode> _symbolRemapping;
20-
21-
public static ObjectDataInterner Null { get; } = new ObjectDataInterner(genericsOnly: false) { _symbolRemapping = new() };
17+
private int _previousHashCount;
2218

23-
public ObjectDataInterner(bool genericsOnly)
19+
public MethodBodyDeduplicator(bool genericsOnly)
2420
{
2521
_genericsOnly = genericsOnly;
2622
}
2723

2824
public bool CanFold(MethodDesc method)
2925
{
30-
if (this == Null)
31-
return false;
32-
3326
if (!_genericsOnly || method.HasInstantiation || method.OwningType.HasInstantiation)
3427
return true;
3528

@@ -39,63 +32,36 @@ public bool CanFold(MethodDesc method)
3932
return false;
4033
}
4134

42-
private void EnsureMap(NodeFactory factory)
35+
public void DeduplicatePass(NodeFactory factory, Dictionary<ISymbolNode, ISymbolNode> previousSymbolRemapping, Dictionary<ISymbolNode, ISymbolNode> symbolRemapping)
4336
{
44-
Debug.Assert(factory.MarkingComplete);
45-
46-
if (_symbolRemapping != null)
47-
return;
48-
49-
HashSet<MethodInternKey> previousMethodHash;
50-
HashSet<MethodInternKey> methodHash = null;
51-
Dictionary<ISymbolNode, ISymbolNode> previousSymbolRemapping;
52-
Dictionary<ISymbolNode, ISymbolNode> symbolRemapping = null;
37+
var methodHash = new HashSet<MethodInternKey>(_previousHashCount, new MethodInternComparer(factory, previousSymbolRemapping, _genericsOnly));
5338

54-
do
39+
foreach (IMethodBodyNode body in factory.MetadataManager.GetCompiledMethodBodies())
5540
{
56-
previousMethodHash = methodHash;
57-
previousSymbolRemapping = symbolRemapping;
58-
methodHash = new HashSet<MethodInternKey>(previousMethodHash?.Count ?? 0, new MethodInternComparer(factory, previousSymbolRemapping, _genericsOnly));
59-
symbolRemapping = new Dictionary<ISymbolNode, ISymbolNode>((int)(1.05 * (previousSymbolRemapping?.Count ?? 0)));
41+
if (!CanFold(body.Method))
42+
continue;
6043

61-
foreach (IMethodBodyNode body in factory.MetadataManager.GetCompiledMethodBodies())
62-
{
63-
if (!CanFold(body.Method))
64-
continue;
65-
66-
// We don't track special unboxing thunks as virtual method use related so ignore them
67-
if (body is ISpecialUnboxThunkNode unboxThunk && unboxThunk.IsSpecialUnboxingThunk)
68-
continue;
44+
// We don't track special unboxing thunks as virtual method use related so ignore them
45+
if (body is ISpecialUnboxThunkNode unboxThunk && unboxThunk.IsSpecialUnboxingThunk)
46+
continue;
6947

70-
// Bodies that are visible from outside should not be folded because we don't know
71-
// if they're address taken.
72-
if (!factory.GetSymbolAlternateName(body, out _).IsNull)
73-
continue;
48+
// Bodies that are visible from outside should not be folded because we don't know
49+
// if they're address taken.
50+
if (!factory.GetSymbolAlternateName(body, out _).IsNull)
51+
continue;
7452

75-
var key = new MethodInternKey(body, factory);
76-
if (methodHash.TryGetValue(key, out MethodInternKey found))
77-
{
78-
symbolRemapping.Add(body, found.Method);
79-
}
80-
else
81-
{
82-
methodHash.Add(key);
83-
}
53+
var key = new MethodInternKey(body, factory);
54+
if (methodHash.TryGetValue(key, out MethodInternKey found))
55+
{
56+
symbolRemapping.TryAdd(body, found.Method);
8457
}
85-
} while (previousSymbolRemapping == null || previousSymbolRemapping.Count < symbolRemapping.Count);
86-
87-
_symbolRemapping = symbolRemapping;
88-
}
89-
90-
public ISymbolNode GetDeduplicatedSymbol(NodeFactory factory, ISymbolNode original)
91-
{
92-
EnsureMap(factory);
93-
94-
ISymbolNode target = original;
95-
if (target is ISymbolNodeWithLinkage symbolWithLinkage)
96-
target = symbolWithLinkage.NodeForLinkage(factory);
58+
else
59+
{
60+
methodHash.Add(key);
61+
}
62+
}
9763

98-
return _symbolRemapping.TryGetValue(target, out ISymbolNode result) ? result : original;
64+
_previousHashCount = methodHash.Count;
9965
}
10066

10167
private sealed class MethodInternKey
@@ -218,7 +184,7 @@ public bool Equals(MethodInternKey a, MethodInternKey b)
218184
if (o1eh == o2eh)
219185
return true;
220186

221-
if (o1eh == null || o2eh == null)
187+
if (o1eh is null || o2eh is null)
222188
return false;
223189

224190
return AreSame(o1eh.GetData(_factory, relocsOnly: false), o2eh.GetData(_factory, relocsOnly: false));

‎src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,8 @@
503503
<Compile Include="Compiler\DependencyAnalysis\ImportedNodeProvider.cs" />
504504
<Compile Include="Compiler\ExpectedIsaFeaturesRootProvider.cs" />
505505
<Compile Include="Compiler\ExternSymbolMappedField.cs" />
506-
<Compile Include="Compiler\ObjectDataInterner.cs" />
506+
<Compile Include="Compiler\MethodBodyDeduplicator.cs" />
507+
<Compile Include="..\..\Common\Compiler\ObjectDataInterner.cs" Link="Compiler\ObjectDataInterner.cs" />
507508
<Compile Include="Compiler\ReachabilityInstrumentationFilter.cs" />
508509
<Compile Include="Compiler\ReachabilityInstrumentationProvider.cs" />
509510
<Compile Include="Compiler\SourceLinkWriter.cs" />

‎src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/CopiedMethodILDeduplicator.cs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
7+
using ILCompiler.DependencyAnalysis;
8+
9+
namespace ILCompiler.DependencyAnalysis.ReadyToRun
10+
{
11+
public sealed class CopiedMethodILDeduplicator : IObjectDataDeduplicator
12+
{
13+
private readonly Func<IEnumerable<CopiedMethodILNode>> _nodesProvider;
14+
private Dictionary<ISymbolNode, ISymbolNode> _cachedMapping;
15+
16+
public CopiedMethodILDeduplicator(Func<IEnumerable<CopiedMethodILNode>> nodesProvider)
17+
{
18+
_nodesProvider = nodesProvider;
19+
}
20+
21+
public void DeduplicatePass(NodeFactory factory, Dictionary<ISymbolNode, ISymbolNode> previousSymbolRemapping, Dictionary<ISymbolNode, ISymbolNode> symbolRemapping)
22+
{
23+
if (_cachedMapping is null)
24+
{
25+
_cachedMapping = new Dictionary<ISymbolNode, ISymbolNode>();
26+
27+
var sortedNodes = new List<CopiedMethodILNode>(_nodesProvider());
28+
sortedNodes.Sort(CompilerComparer.Instance);
29+
30+
var hashSet = new HashSet<InternKey>(new InternComparer());
31+
32+
foreach (CopiedMethodILNode node in sortedNodes)
33+
{
34+
// No need to deduplicate unmarked nodes.
35+
// They won't be emitted anyway.
36+
if (!node.Marked)
37+
{
38+
continue;
39+
}
40+
var key = new InternKey(node, factory);
41+
if (hashSet.TryGetValue(key, out InternKey existing))
42+
{
43+
_cachedMapping[node] = existing.Node;
44+
}
45+
else
46+
{
47+
hashSet.Add(key);
48+
}
49+
}
50+
}
51+
52+
foreach (KeyValuePair<ISymbolNode, ISymbolNode> entry in _cachedMapping)
53+
{
54+
symbolRemapping.TryAdd(entry.Key, entry.Value);
55+
}
56+
}
57+
58+
private sealed class InternKey
59+
{
60+
public CopiedMethodILNode Node { get; }
61+
public int HashCode { get; }
62+
public byte[] Data { get; }
63+
64+
public InternKey(CopiedMethodILNode node, NodeFactory factory)
65+
{
66+
Node = node;
67+
68+
Data = node.GetData(factory, relocsOnly: false).Data;
69+
var hashCode = new HashCode();
70+
hashCode.AddBytes(Data);
71+
HashCode = hashCode.ToHashCode();
72+
}
73+
}
74+
75+
private sealed class InternComparer : IEqualityComparer<InternKey>
76+
{
77+
public int GetHashCode(InternKey key) => key.HashCode;
78+
79+
public bool Equals(InternKey a, InternKey b)
80+
{
81+
if (a.HashCode != b.HashCode)
82+
return false;
83+
84+
return a.Data.AsSpan().SequenceEqual(b.Data);
85+
}
86+
}
87+
}
88+
}

0 commit comments

Comments
 (0)