Skip to content
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.

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

Optimize relocation counting in DehydratedDataNode.GetData #103202

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

filipnavara
Copy link
Member

Save a couple of duplicate dictionary lookups. Thanks @MihaZupan for the idea.

Profile before:

  100%   GetData  •  5,673 ms  •  ILCompiler.DependencyAnalysis.DehydratedDataNode.GetData(NodeFactory, Boolean)
  ► 72.9%   TryInsert  •  4,136 ms  •  System.Collections.Generic.Dictionary`2.TryInsert(TKey, TValue, InsertionBehavior)
  ► 8.85%   FindValue  •  502 ms  •  System.Collections.Generic.Dictionary`2.FindValue(TKey)
  ► 7.11%   NecessaryTypeSymbol  •  403 ms  •  ILCompiler.DependencyAnalysis.NodeFactory.NecessaryTypeSymbol(TypeDesc)
    2.55%   IsInstanceOfInterface  •  145 ms  •  System.Runtime.CompilerServices.CastHelpers.IsInstanceOfInterface(Void*, Object)
  ► 2.15%   Sort  •  122 ms  •  System.Collections.Generic.ArraySortHelper`1.Sort(Span, Comparison)
    0.83%   Copy  •  47 ms  •  System.Array.Copy(Array, Int32, Array, Int32, Int32)
    0.83%   [Unknown]  •  47 ms
  ► 0.38%   EmitReloc  •  22 ms  •  ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitReloc(ISymbolNode, RelocType, Int32)
  ► 0.37%   MoveNext  •  21 ms  •  ILCompiler.MetadataManager+<GetDehydratableData>d__42.MoveNext()
    0.23%   AddRange  •  13 ms  •  System.Collections.Generic.Dictionary`2.AddRange(IEnumerable)
    0.16%   GetSize  •  9.0 ms  •  ILCompiler.DependencyAnalysis.Relocation.GetSize(RelocType)
  ► 0.15%   Grow  •  8.8 ms  •  System.Collections.Generic.ArrayBuilder`1.Grow(Int32)
  ► 0.14%   ToObjectData  •  7.8 ms  •  ILCompiler.DependencyAnalysis.ObjectDataBuilder.ToObjectData()
    0.13%   ReadValue  •  7.2 ms  •  ILCompiler.DependencyAnalysis.Relocation.ReadValue(RelocType, Void*)
    0.11%   List`1..ctor  •  6.4 ms  •  System.Collections.Generic.List`1..ctor(IEnumerable)
    0.11%   Encode  •  6.0 ms  •  Internal.Runtime.DehydratedDataCommand.Encode(Int32, Int32, Byte[])
    0.06%   coreclr.dll  •  3.6 ms
    0.02%   ToArray  •  1.4 ms  •  System.Collections.Generic.List`1.ToArray()
    0.02%   NodeForLinkage  •  1.0 ms  •  ILCompiler.DependencyAnalysis.EETypeNode.NodeForLinkage(NodeFactory)
    0.02%   ntoskrnl.exe  •  0.9 ms
    <0.01%   Dictionary`2..ctor  •  0.07 ms  •  System.Collections.Generic.Dictionary`2..ctor(Int32, IEqualityComparer)
    <0.01%   Resize  •  0.0000002 ms  •  System.Array.Resize(ref T[], Int32)

Profile after:

  100%   GetData  •  5,563 ms  •  ILCompiler.DependencyAnalysis.DehydratedDataNode.GetData(NodeFactory, Boolean)
    72.6%   EmitReloc  •  4,041 ms  •  ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitReloc(ISymbolNode, RelocType, Int32)
      72.2%   Grow  •  4,015 ms  •  System.Collections.Generic.ArrayBuilder`1.Grow(Int32)
        72.2%   Resize  •  4,015 ms  •  System.Array.Resize(ref T[], Int32)
      0.07%   Resize  •  4.0 ms  •  System.Array.Resize(ref T[], Int32)
      <0.01%   [Unknown]  •  0.5 ms
  ► 6.78%   GetValueRefOrAddDefault  •  377 ms  •  System.Collections.Generic.Dictionary`2+CollectionsMarshalHelper.GetValueRefOrAddDefault(Dictionary, TKey, out Boolean)
  ► 5.94%   NecessaryTypeSymbol  •  331 ms  •  ILCompiler.DependencyAnalysis.NodeFactory.NecessaryTypeSymbol(TypeDesc)
  ► 4.16%   FindValue  •  232 ms  •  System.Collections.Generic.Dictionary`2.FindValue(TKey)
    2.41%   IsInstanceOfInterface  •  134 ms  •  System.Runtime.CompilerServices.CastHelpers.IsInstanceOfInterface(Void*, Object)
  ► 2.03%   Sort  •  113 ms  •  System.Collections.Generic.ArraySortHelper`1.Sort(Span, Comparison)
    0.71%   [Unknown]  •  39 ms
    0.63%   Copy  •  35 ms  •  System.Array.Copy(Array, Int32, Array, Int32, Int32)
  ► 0.45%   MoveNext  •  25 ms  •  ILCompiler.MetadataManager+<GetDehydratableData>d__42.MoveNext()
    0.34%   ReadValue  •  19 ms  •  ILCompiler.DependencyAnalysis.Relocation.ReadValue(RelocType, Void*)
    0.24%   AddRange  •  13 ms  •  System.Collections.Generic.Dictionary`2.AddRange(IEnumerable)
  ► 0.21%   Encode  •  12 ms  •  Internal.Runtime.DehydratedDataCommand.Encode(Int32, Int32, Byte[])
    0.18%   GetSize  •  10 ms  •  ILCompiler.DependencyAnalysis.Relocation.GetSize(RelocType)
  ► 0.14%   ToObjectData  •  8.0 ms  •  ILCompiler.DependencyAnalysis.ObjectDataBuilder.ToObjectData()
  ► 0.11%   Grow  •  6.0 ms  •  System.Collections.Generic.ArrayBuilder`1.Grow(Int32)
    0.09%   NodeForLinkage  •  5.0 ms  •  ILCompiler.DependencyAnalysis.EETypeNode.NodeForLinkage(NodeFactory)
    0.08%   coreclr.dll  •  4.3 ms
    0.06%   ToArray  •  3.1 ms  •  System.Collections.Generic.List`1.ToArray()
    0.01%   ntoskrnl.exe  •  0.6 ms
    <0.01%   List`1..ctor  •  0.5 ms  •  System.Collections.Generic.List`1..ctor(IEnumerable)
    <0.01%   Dictionary`2..ctor  •  0.09 ms  •  System.Collections.Generic.Dictionary`2..ctor(Int32, IEqualityComparer)
    <0.01%   Resize  •  0.0000002 ms  •  System.Array.Resize(ref T[], Int32)

The actual saving is around 4.2s or 70+% of the method runtime (TryInsert + FindValue vs. GetValueRefOrAddDefault). It seems it just happened to hit a GC in the second profile.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jun 8, 2024
Copy link
Contributor

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

@filipnavara filipnavara closed this Jun 8, 2024
@filipnavara
Copy link
Member Author

I think there's a bug in the code. The changed profile is not a coincidence.

@filipnavara
Copy link
Member Author

I think there's a bug in the code. The changed profile is not a coincidence.

I re-run the code twice, checking that the results of the original and new code are identical. There were no diffs.

Re-running the profiler produces this:

  100%   GetData  •  1,556 ms  •  ILCompiler.DependencyAnalysis.DehydratedDataNode.GetData(NodeFactory, Boolean)
  ► 23.4%   GetValueRefOrAddDefault  •  365 ms  •  System.Collections.Generic.Dictionary`2+CollectionsMarshalHelper.GetValueRefOrAddDefault(Dictionary, TKey, out Boolean)
  ► 21.7%   NecessaryTypeSymbol  •  337 ms  •  ILCompiler.DependencyAnalysis.NodeFactory.NecessaryTypeSymbol(TypeDesc)
  ► 17.0%   FindValue  •  264 ms  •  System.Collections.Generic.Dictionary`2.FindValue(TKey)
    7.60%   IsInstanceOfInterface  •  118 ms  •  System.Runtime.CompilerServices.CastHelpers.IsInstanceOfInterface(Void*, Object)
  ► 7.59%   Sort  •  118 ms  •  System.Collections.Generic.ArraySortHelper`1.Sort(Span, Comparison)
    3.51%   Copy  •  55 ms  •  System.Array.Copy(Array, Int32, Array, Int32, Int32)
    1.69%   [Unknown]  •  26 ms
  ► 1.47%   EmitReloc  •  23 ms  •  ILCompiler.DependencyAnalysis.ObjectDataBuilder.EmitReloc(ISymbolNode, RelocType, Int32)
  ► 1.43%   MoveNext  •  22 ms  •  ILCompiler.MetadataManager+<GetDehydratableData>d__42.MoveNext()
    1.14%   AddRange  •  18 ms  •  System.Collections.Generic.Dictionary`2.AddRange(IEnumerable)
  ► 1.02%   Grow  •  16 ms  •  System.Collections.Generic.ArrayBuilder`1.Grow(Int32)
    0.86%   Encode  •  13 ms  •  Internal.Runtime.DehydratedDataCommand.Encode(Int32, Int32, Byte[])
  ► 0.44%   ToObjectData  •  6.9 ms  •  ILCompiler.DependencyAnalysis.ObjectDataBuilder.ToObjectData()
    0.39%   GetSize  •  6.0 ms  •  ILCompiler.DependencyAnalysis.Relocation.GetSize(RelocType)
    0.39%   ReadValue  •  6.0 ms  •  ILCompiler.DependencyAnalysis.Relocation.ReadValue(RelocType, Void*)
    0.28%   coreclr.dll  •  4.4 ms
    0.28%   List`1..ctor  •  4.3 ms  •  System.Collections.Generic.List`1..ctor(IEnumerable)
    0.20%   ToArray  •  3.2 ms  •  System.Collections.Generic.List`1.ToArray()
    0.13%   NodeForLinkage  •  2.0 ms  •  ILCompiler.DependencyAnalysis.EETypeNode.NodeForLinkage(NodeFactory)
    0.06%   NodeForLinkage  •  1.0 ms  •  ILCompiler.DependencyAnalysis.TentativeMethodNode.NodeForLinkage(NodeFactory)
    0.06%   get_Current  •  1.0 ms  •  ILCompiler.MetadataManager+<GetDehydratableData>d__42.get_Current()
    0.05%   ntoskrnl.exe  •  0.8 ms
    <0.01%   Dictionary`2..ctor  •  0.08 ms  •  System.Collections.Generic.Dictionary`2..ctor(Int32, IEqualityComparer)
    <0.01%   Resize  •  0.0000002 ms  •  System.Array.Resize(ref T[], Int32)

Must have been some profiler fluke.

@filipnavara filipnavara reopened this Jun 8, 2024
Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Thanks!

@MichalStrehovsky MichalStrehovsky merged commit c1cebef into dotnet:main Jun 10, 2024
87 checks passed
@filipnavara filipnavara deleted the dehydict branch June 10, 2024 06:17
@github-actions github-actions bot locked and limited conversation to collaborators Jul 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants