Skip to content

Commit

Permalink
Code Review Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
cshung committed Jan 6, 2023
1 parent 3c3bcb3 commit 70d63ae
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 74 deletions.
16 changes: 10 additions & 6 deletions src/PerfView/GcStats.cs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,11 @@ public static void ToXml(TextWriter writer, TraceProcess stats, TraceLoadedDotNe
writer.Write(" Gen0MinBudgetConfig=\"{0}\"", runtime.GC.GCSettings.Gen0MinBudgetConfig);
writer.Write(" Gen0MaxBudgetConfig=\"{0}\"", runtime.GC.GCSettings.Gen0MaxBudgetConfig);
writer.Write(" HighMemPercentConfig=\"{0}\"", runtime.GC.GCSettings.HighMemPercentConfig);
writer.Write(" BitSettings=\"{0}\"", runtime.GC.GCSettings.BitSettings);
writer.Write(" GCSettingsConcurrent=\"{0}\"", runtime.GC.GCSettings.BitSettings.HasFlag(GCSettingsFlags.GCSettingsConcurrent));
writer.Write(" GCSettingsLargePages=\"{0}\"", runtime.GC.GCSettings.BitSettings.HasFlag(GCSettingsFlags.GCSettingsLargePages));
writer.Write(" GCSettingsFrozenSegs=\"{0}\"", runtime.GC.GCSettings.BitSettings.HasFlag(GCSettingsFlags.GCSettingsFrozenSegs));
writer.Write(" GCSettingsHardLimitConfig=\"{0}\"", runtime.GC.GCSettings.BitSettings.HasFlag(GCSettingsFlags.GCSettingsHardLimitConfig));
writer.Write(" GCSettingsNoAffinitize=\"{0}\"", runtime.GC.GCSettings.BitSettings.HasFlag(GCSettingsFlags.GCSettingsNoAffinitize));
}
if (stats.CPUMSec != 0)
{
Expand Down Expand Up @@ -549,11 +553,11 @@ public static void ToXmlAttribs(TextWriter writer, TraceProcess stats, TraceLoad
if (gc.LOHCompactInfos.Count > 0)
{
GCLOHCompactInfo lohCompactInfo = gc.LOHCompactInfos[HeapNum];
writer.Write(" LohTimePlan =\"{0:n3}\" ", lohCompactInfo.TimePlan);
writer.Write(" LohTimeCompact =\"{0:n3}\" ", lohCompactInfo.TimeCompact);
writer.Write(" LohTimeRelocate =\"{0:n3}\" ", lohCompactInfo.TimeRelocate);
writer.Write(" LohTotalRefs =\"{0}\" ", lohCompactInfo.TotalRefs);
writer.Write(" LohZeroRefs =\"{0}\" ", lohCompactInfo.ZeroRefs);
writer.Write(" LOHTimePlan =\"{0:n3}\" ", lohCompactInfo.TimePlan);
writer.Write(" LOHTimeCompact =\"{0:n3}\" ", lohCompactInfo.TimeCompact);
writer.Write(" LOHTimeRelocate =\"{0:n3}\" ", lohCompactInfo.TimeRelocate);
writer.Write(" LOHTotalRefs =\"{0}\" ", lohCompactInfo.TotalRefs);
writer.Write(" LOHZeroRefs =\"{0}\" ", lohCompactInfo.ZeroRefs);
}
}
else
Expand Down
6 changes: 3 additions & 3 deletions src/TraceEvent/Computers/TraceManagedProcess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1811,9 +1811,9 @@ public class GCSettings
public long Gen0MinBudgetConfig { get { return m_Gen0MinBudgetConfig; } }
public long Gen0MaxBudgetConfig { get { return m_Gen0MaxBudgetConfig; } }
public int HighMemPercentConfig { get { return m_HighMemPercentConfig; } }
public int BitSettings { get { return m_BitSettings; } }
public GCSettingsFlags BitSettings { get { return m_BitSettings; } }

public GCSettings(long HardLimit, long LOHThreshold, long PhysicalMemoryConfig, long Gen0MinBudgetConfig, long Gen0MaxBudgetConfig, int HighMemPercentConfig, int BitSettings)
public GCSettings(long HardLimit, long LOHThreshold, long PhysicalMemoryConfig, long Gen0MinBudgetConfig, long Gen0MaxBudgetConfig, int HighMemPercentConfig, GCSettingsFlags BitSettings)
{
m_HardLimit = HardLimit;
m_LOHThreshold = LOHThreshold;
Expand All @@ -1830,7 +1830,7 @@ public GCSettings(long HardLimit, long LOHThreshold, long PhysicalMemoryConfig,
private long m_Gen0MinBudgetConfig;
private long m_Gen0MaxBudgetConfig;
private int m_HighMemPercentConfig;
private int m_BitSettings;
private GCSettingsFlags m_BitSettings;
}

/// <summary>
Expand Down
151 changes: 86 additions & 65 deletions src/TraceEvent/Parsers/ClrTraceEventParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2638,58 +2638,44 @@ public sealed class GCLOHCompactInfo
/// <summary>
/// The time spent on planing the LOH for compaction
/// </summary>
public int TimePlan
{
get
{
return m_timePlan;

}
}
public int TimePlan { get { return m_timePlan; } }

/// <summary>
/// The time spent on copying objects during LOH compaction
/// </summary>
public int TimeCompact
{
get
{
return m_timeCompact;

}
}
public int TimeCompact { get { return m_timeCompact; } }

/// <summary>
/// The time spent on relocating pointers during LOH compaction
/// </summary>
public int TimeRelocate
{
get
{
return m_timeRelocate;
}
}
public int TimeRelocate { get { return m_timeRelocate; } }

/// <summary>
/// The total number of pointers found in the LOH during LOH compaction
/// </summary>
public long TotalRefs
{
get
{
return m_totalRefs;
}
}
public long TotalRefs { get { return m_totalRefs; } }

/// <summary>
/// The total number of pointers found pointing to null in the LOH during LOH compaction
/// </summary>
public long ZeroRefs
public long ZeroRefs { get { return m_zeroRefs; } }

public override string ToString()
{
get
{
return m_zeroRefs;
}
StringBuilder sb = new StringBuilder();
return ToXml(sb).ToString();
}

public StringBuilder ToXml(StringBuilder sb)
{
sb.Append(" <GCLOHCompactInfo ");
TraceEvent.XmlAttrib(sb, "TimePlan", TimePlan);
TraceEvent.XmlAttrib(sb, "TimeCompact", TimeCompact);
TraceEvent.XmlAttrib(sb, "TimeRelocate", TimeRelocate);
TraceEvent.XmlAttrib(sb, "TotalRefs", TotalRefs);
TraceEvent.XmlAttrib(sb, "ZeroRefs", ZeroRefs);
sb.Append("/>");
return sb;
}

#region private
Expand All @@ -2710,7 +2696,7 @@ internal GCLOHCompactInfo(int timePlan, int timeCompact, int timeRelocate, long
#endregion
}

public class GCLOHCompactTraceData : TraceEvent
public sealed class GCLOHCompactTraceData : TraceEvent
{
public int ClrInstanceID { get { return GetInt16At(0); } }

Expand Down Expand Up @@ -2752,7 +2738,12 @@ public override StringBuilder ToXml(StringBuilder sb)
Prefix(sb);
XmlAttrib(sb, "ClrInstanceID", ClrInstanceID);
XmlAttrib(sb, "Count", Count);
sb.Append("/>");
sb.AppendLine(">");
for (int i = 0; i < Count; i++)
{
Info(i).ToXml(sb).AppendLine();
}
sb.Append("</Event>");
return sb;
}

Expand Down Expand Up @@ -2780,6 +2771,13 @@ public override object PayloadValue(int index)
}
}

// <struct name="Values" count="Count" >
// <data name="TimePlan" inType="win:UInt32" />
// <data name="TimeCompact" inType="win:UInt32" />
// <data name="TimeRelocate" inType="win:UInt32" />
// <data name="TotalRefs" inType="win:Pointer" />
// <data name="ZeroRefs" inType="win:Pointer" />
// </struct>
private int SizeOfGCLOHCompactInfo
{
get
Expand All @@ -2792,7 +2790,7 @@ private int SizeOfGCLOHCompactInfo
#endregion
}

public class GCFitBucket
public sealed class GCFitBucket
{
#region private
internal GCFitBucket(int index, int count, long size)
Expand All @@ -2805,34 +2803,32 @@ internal GCFitBucket(int index, int count, long size)
/// <summary>
/// The index of the bucket. This is required because bucket of zero size is not reported.
/// </summary>
public int Index
{
get
{
return m_index;
}
}
public int Index { get { return m_index; } }

/// <summary>
/// The number of items in the bucket.
/// </summary>
public int Count
{
get
{
return m_count;
}
}
public int Count { get { return m_count; } }

/// <summary>
/// The total size of items in the bucket.
/// </summary>
public long Size
public long Size { get { return m_size; } }

public override string ToString()
{
get
{
return m_size;
}
StringBuilder sb = new StringBuilder();
return ToXml(sb).ToString();
}

public StringBuilder ToXml(StringBuilder sb)
{
sb.Append(" <GCFitBucket ");
TraceEvent.XmlAttrib(sb, "Index", Index);
TraceEvent.XmlAttrib(sb, "Count", Count);
TraceEvent.XmlAttrib(sb, "Size", Size);
sb.Append("/>");
return sb;
}

private int m_index;
Expand All @@ -2854,13 +2850,13 @@ public enum BucketKind

/// <summary>
/// When the BucketKind is PlugsInCondemned, the buckets in the associated GCFitBucketInfo events
/// represents plugs that we found during the plan phase of a Gen1 GC where it is planned to fit in the
/// current (i.e. condemned) generation.
/// represents plugs that we found during the plan phase of a Gen1 GC where it is planned to fit in the
/// current (i.e. condemned) generation.
/// </summary>
PlugsInCondemned = 1
}

public class GCFitBucketInfoTraceData : TraceEvent
public sealed class GCFitBucketInfoTraceData : TraceEvent
{
public int ClrInstanceID { get { return GetInt16At(0); } }

Expand Down Expand Up @@ -2907,9 +2903,9 @@ protected internal override Delegate Target
}
protected internal override void Validate()
{
int size0 = 14 + Count * SizeOfGCFitBucket;
Debug.Assert(!(Version == 0 && EventDataLength != size0));
Debug.Assert(!(Version > 0 && EventDataLength < size0));
int size = 14 + Count * SizeOfGCFitBucket;
Debug.Assert(!(Version == 0 && EventDataLength != size));
Debug.Assert(!(Version > 0 && EventDataLength < size));
}
public override StringBuilder ToXml(StringBuilder sb)
{
Expand All @@ -2918,7 +2914,12 @@ public override StringBuilder ToXml(StringBuilder sb)
XmlAttrib(sb, "BucketKind", BucketKind);
XmlAttrib(sb, "TotalSize", TotalSize);
XmlAttrib(sb, "Count", Count);
sb.Append("/>");
sb.AppendLine(">");
for (int i = 0; i < Count; i++)
{
Buckets(i).ToXml(sb).AppendLine();
}
sb.Append("</Event>");
return sb;
}

Expand Down Expand Up @@ -2954,6 +2955,11 @@ public override object PayloadValue(int index)
}
}

// <struct name="Values" count="Count" >
// <data name="Index" inType="win:UInt16" />
// <data name="Count" inType="win:UInt32" />
// <data name="Size" inType="win:Pointer" outType="win:HexInt64" />
// </struct>
private int SizeOfGCFitBucket
{
get
Expand Down Expand Up @@ -6974,6 +6980,11 @@ public override StringBuilder ToXml(StringBuilder sb)
XmlAttrib(sb, "Reason", Reason);
XmlAttrib(sb, "GlobalMechanisms", GlobalMechanisms);
XmlAttrib(sb, "ClrInstanceID", ClrInstanceID);
if (Version >= 4)
{
XmlAttrib(sb, "Count", Count);
XmlAttrib(sb, "Times", string.Join(",", Times ?? Array.Empty<int>()));
}
sb.Append("/>");
return sb;
}
Expand Down Expand Up @@ -9918,6 +9929,16 @@ public override object PayloadValue(int index)
#endregion
}

[Flags]
public enum GCSettingsFlags : int
{
GCSettingsConcurrent = 0x00000001,
GCSettingsLargePages = 0x00000002,
GCSettingsFrozenSegs = 0x00000004,
GCSettingsHardLimitConfig = 0x00000008,
GCSettingsNoAffinitize = 0x00000010,
};

public sealed class GCSettingsRundownTraceData : TraceEvent
{
public long HardLimit { get { return GetInt64At(0); } }
Expand All @@ -9926,7 +9947,7 @@ public sealed class GCSettingsRundownTraceData : TraceEvent
public long Gen0MinBudgetConfig { get { return GetInt64At(24); } }
public long Gen0MaxBudgetConfig { get { return GetInt64At(32); } }
public int HighMemPercentConfig { get { return GetInt32At(40); } }
public int BitSettings { get { return GetInt32At(44); } }
public GCSettingsFlags BitSettings { get { return (GCSettingsFlags)GetInt32At(44); } }
public int ClrInstanceID { get { return GetInt16At(48); } }

#region Private
Expand Down

0 comments on commit 70d63ae

Please sign in to comment.