From 70d63ae0d0d6eb8ee98acbde3e4c7fb1c9afbd8a Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Fri, 6 Jan 2023 09:03:06 -0800 Subject: [PATCH] Code Review Feedback --- src/PerfView/GcStats.cs | 16 +- .../Computers/TraceManagedProcess.cs | 6 +- src/TraceEvent/Parsers/ClrTraceEventParser.cs | 151 ++++++++++-------- 3 files changed, 99 insertions(+), 74 deletions(-) diff --git a/src/PerfView/GcStats.cs b/src/PerfView/GcStats.cs index fbb8eaf4b..c3e939316 100644 --- a/src/PerfView/GcStats.cs +++ b/src/PerfView/GcStats.cs @@ -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) { @@ -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 diff --git a/src/TraceEvent/Computers/TraceManagedProcess.cs b/src/TraceEvent/Computers/TraceManagedProcess.cs index 97a64dcfd..c2cc9862d 100644 --- a/src/TraceEvent/Computers/TraceManagedProcess.cs +++ b/src/TraceEvent/Computers/TraceManagedProcess.cs @@ -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; @@ -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; } /// diff --git a/src/TraceEvent/Parsers/ClrTraceEventParser.cs b/src/TraceEvent/Parsers/ClrTraceEventParser.cs index 6d5ba0777..905eee302 100644 --- a/src/TraceEvent/Parsers/ClrTraceEventParser.cs +++ b/src/TraceEvent/Parsers/ClrTraceEventParser.cs @@ -2638,58 +2638,44 @@ public sealed class GCLOHCompactInfo /// /// The time spent on planing the LOH for compaction /// - public int TimePlan - { - get - { - return m_timePlan; - - } - } + public int TimePlan { get { return m_timePlan; } } /// /// The time spent on copying objects during LOH compaction /// - public int TimeCompact - { - get - { - return m_timeCompact; - - } - } + public int TimeCompact { get { return m_timeCompact; } } /// /// The time spent on relocating pointers during LOH compaction /// - public int TimeRelocate - { - get - { - return m_timeRelocate; - } - } + public int TimeRelocate { get { return m_timeRelocate; } } /// /// The total number of pointers found in the LOH during LOH compaction /// - public long TotalRefs - { - get - { - return m_totalRefs; - } - } + public long TotalRefs { get { return m_totalRefs; } } /// /// The total number of pointers found pointing to null in the LOH during LOH compaction /// - 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(" "); + return sb; } #region private @@ -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); } } @@ -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(""); return sb; } @@ -2780,6 +2771,13 @@ public override object PayloadValue(int index) } } + // + // + // + // + // + // + // private int SizeOfGCLOHCompactInfo { get @@ -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) @@ -2805,34 +2803,32 @@ internal GCFitBucket(int index, int count, long size) /// /// The index of the bucket. This is required because bucket of zero size is not reported. /// - public int Index - { - get - { - return m_index; - } - } + public int Index { get { return m_index; } } /// /// The number of items in the bucket. /// - public int Count - { - get - { - return m_count; - } - } + public int Count { get { return m_count; } } /// /// The total size of items in the bucket. /// - 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(" "); + return sb; } private int m_index; @@ -2854,13 +2850,13 @@ public enum BucketKind /// /// 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. /// PlugsInCondemned = 1 } - public class GCFitBucketInfoTraceData : TraceEvent + public sealed class GCFitBucketInfoTraceData : TraceEvent { public int ClrInstanceID { get { return GetInt16At(0); } } @@ -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) { @@ -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(""); return sb; } @@ -2954,6 +2955,11 @@ public override object PayloadValue(int index) } } + // + // + // + // + // private int SizeOfGCFitBucket { get @@ -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())); + } sb.Append("/>"); return sb; } @@ -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); } } @@ -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