Skip to content

Commit

Permalink
Cache default heap dom sizes once computed to avoid slowing down some…
Browse files Browse the repository at this point in the history
… workflows

Summary:
Running a command like
```
lo 'type "Foo" 'exec "bt 'dom"
```
saw a noticeable slowdown with the previous change. Cache the default HeapDomSizes object to avoid recomputing it every time.

Differential Revision: D54607644

fbshipit-source-id: bb3aac3fb38d123781a5b1d833798ea35e4eded0
  • Loading branch information
elliekorn authored and facebook-github-bot committed Mar 7, 2024
1 parent dd85953 commit 83f3bb5
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
14 changes: 14 additions & 0 deletions Analysis/HeapDom.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public sealed class HeapDom
readonly int[] m_doms;
readonly Dictionary<int, List<int>> m_domTree;
readonly int m_numberOfNonLeafNodes;
HeapDomSizes? m_defaultHeapDomSizes;

public HeapDom(IBacktracer backtracer)
{
Expand Down Expand Up @@ -43,6 +44,19 @@ public int GetDominator(int nodeIndex)
return children;
}

public HeapDomSizes DefaultHeapDomSizes
{
get
{
if (m_defaultHeapDomSizes == null)
{
m_defaultHeapDomSizes = new HeapDomSizes(this, typeSet: null);
}

return m_defaultHeapDomSizes;
}
}

int[] ComputeDominators()
{
// Engineered algorithm from https://www.cs.rice.edu/~keith/EMBED/dom.pdf
Expand Down
8 changes: 3 additions & 5 deletions CommandInfrastructure/Command.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,15 @@ public HeapDom CurrentHeapDom

public HeapDomSizes MakeHeapDomSizes(CommandLineArgument? typeIndexOrPattern, bool includeDerived)
{
TypeSet? typeSet;
if (typeIndexOrPattern != null)
{
typeSet = typeIndexOrPattern.ResolveTypeIndexOrPattern(Context, includeDerived);
TypeSet typeSet = typeIndexOrPattern.ResolveTypeIndexOrPattern(Context, includeDerived);
return new HeapDomSizes(CurrentHeapDom, typeSet);
}
else
{
typeSet = null;
return CurrentHeapDom.DefaultHeapDomSizes;
}

return new HeapDomSizes(CurrentHeapDom, typeSet);
}

public void DescribeAddress(NativeWord addressOfValue, StringBuilder sb)
Expand Down
4 changes: 2 additions & 2 deletions Commands/ListObjectsCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ internal Selection(Context context, SortOrder sortOrder)
m_sortOrder = sortOrder;
if (m_sortOrder == SortOrder.SortByDomSize)
{
// We use a null typeSet here because the existing 'type argument is not what we want.
// We use the default HeapDomSizes here because the existing 'type argument is not what we want.
// If we want to support sorting by dominator size for a type-filtered dominator tree,
// we can decide to add 'domtype and 'domincludederived arguments.
m_heapDomSizes = new HeapDomSizes(m_context.CurrentHeapDom!, typeSet: null);
m_heapDomSizes = m_context.CurrentHeapDom!.DefaultHeapDomSizes;
}

if (sortOrder == SortOrder.SortByDomSize)
Expand Down

0 comments on commit 83f3bb5

Please sign in to comment.