Skip to content

Commit 985eedd

Browse files
jakobbotschjakobbotsch
authored
SPMI: Add and utilize number of contexts with diffs (#76011)
Previously, if there was any diff in a collection, that collection would be shown in all tables (i.e. Overall, FullOpts, MinOpts). The main reason was that we only had "has diffs" information on a per-collection basis, not for each of the categories. This changes SPMI to communicate back for each category how many contexts had diffs in them, and uses this to hide tables/rows without any diffs, and to show this information under details.
1 parent 05aa1fd commit 985eedd

File tree

4 files changed

+69
-51
lines changed

4 files changed

+69
-51
lines changed

‎src/coreclr/scripts/superpmi.py‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1893,17 +1893,26 @@ def sum_diff(row, col):
18931893
html_color(base_color, "{:,d}".format(missing_base_contexts)),
18941894
html_color(diff_color, "{:,d}".format(missing_diff_contexts))))
18951895

1896-
if any(has_diff for (_, _, _, has_diff, _) in asm_diffs):
1896+
def has_diffs(row):
1897+
return int(row["Contexts with diffs"]) > 0
1898+
1899+
# Exclude entire diffs section?
1900+
if any(has_diffs(diff_metrics["Overall"]) for (_, _, diff_metrics, _, _) in asm_diffs):
18971901
def write_pivot_section(row):
1902+
# Exclude this particular Overall/MinOpts/FullOpts table?
1903+
if not any(has_diffs(diff_metrics[row]) for (_, _, diff_metrics, _, _) in asm_diffs):
1904+
return
1905+
18981906
write_fh.write("\n<details>\n")
18991907
sum_base = sum(int(base_metrics[row]["Diffed code bytes"]) for (_, base_metrics, _, _, _) in asm_diffs)
19001908
sum_diff = sum(int(diff_metrics[row]["Diffed code bytes"]) for (_, _, diff_metrics, _, _) in asm_diffs)
19011909

19021910
write_fh.write("<summary>{} ({} bytes)</summary>\n\n".format(row, format_delta(sum_base, sum_diff)))
19031911
write_fh.write("|Collection|Base size (bytes)|Diff size (bytes)|\n")
19041912
write_fh.write("|---|--:|--:|\n")
1905-
for (mch_file, base_metrics, diff_metrics, has_diffs, _) in asm_diffs:
1906-
if not has_diffs:
1913+
for (mch_file, base_metrics, diff_metrics, _, _) in asm_diffs:
1914+
# Exclude this particular row?
1915+
if not has_diffs(diff_metrics[row]):
19071916
continue
19081917

19091918
write_fh.write("|{}|{:,d}|{}|\n".format(
@@ -1925,14 +1934,15 @@ def write_pivot_section(row):
19251934
write_fh.write("\n<details>\n")
19261935
write_fh.write("<summary>Details</summary>\n\n")
19271936

1928-
write_fh.write("|Collection|Diffed contexts|MinOpts|FullOpts|Missed, base|Missed, diff|\n")
1929-
write_fh.write("|---|--:|--:|--:|--:|--:|\n")
1937+
write_fh.write("|Collection|Diffed contexts|MinOpts|FullOpts|Contexts with diffs|Missed, base|Missed, diff|\n")
1938+
write_fh.write("|---|--:|--:|--:|--:|--:|--:|\n")
19301939
for (mch_file, base_metrics, diff_metrics, has_diffs, jit_analyze_summary_file) in asm_diffs:
1931-
write_fh.write("|{}|{:,d}|{:,d}|{:,d}|{:,d}|{:,d}|\n".format(
1940+
write_fh.write("|{}|{:,d}|{:,d}|{:,d}|{:,d}|{:,d}|{:,d}|\n".format(
19321941
mch_file,
19331942
int(diff_metrics["Overall"]["Successful compiles"]),
19341943
int(diff_metrics["MinOpts"]["Successful compiles"]),
19351944
int(diff_metrics["FullOpts"]["Successful compiles"]),
1945+
int(diff_metrics["Overall"]["Contexts with diffs"]),
19361946
int(base_metrics["Overall"]["Missing compiles"]),
19371947
int(diff_metrics["Overall"]["Missing compiles"])))
19381948

@@ -2159,11 +2169,8 @@ def replay_with_throughput_diff(self):
21592169
# instruction count and all collections.
21602170
def is_significant_pct(base, diff):
21612171
return round((diff - base) / base * 100, 2) != 0
2162-
def is_significant(base, diff):
2163-
def check(col):
2164-
return is_significant_pct(int(base[col]["Diff executed instructions"]), int(diff[col]["Diff executed instructions"]))
2165-
2166-
return check("Overall") or check("MinOpts") or check("FullOpts")
2172+
def is_significant(row, base, diff):
2173+
return is_significant_pct(int(base[row]["Diff executed instructions"]), int(diff[row]["Diff executed instructions"]))
21672174
def format_pct(base_instructions, diff_instructions):
21682175
plus_if_positive = "+" if diff_instructions > base_instructions else ""
21692176
text = "{}{:.2f}%".format(plus_if_positive, (diff_instructions - base_instructions) / base_instructions * 100)
@@ -2173,24 +2180,23 @@ def format_pct(base_instructions, diff_instructions):
21732180

21742181
return text
21752182

2176-
significant_diffs = {}
2177-
for mch_file, base, diff in tp_diffs:
2178-
significant_diffs[mch_file] = is_significant(base, diff)
2183+
if any(is_significant(row, base, diff) for row in ["Overall", "MinOpts", "FullOpts"] for (_, base, diff) in tp_diffs):
2184+
def write_pivot_section(row):
2185+
if not any(is_significant(row, base, diff) for (_, base, diff) in tp_diffs):
2186+
return
21792187

2180-
if any(significant_diffs[mch_file] for (mch_file, _, _) in tp_diffs):
2181-
def write_pivot_section(col):
21822188
write_fh.write("\n<details>\n")
2183-
sum_base = sum(int(base_metrics[col]["Diff executed instructions"]) for (_, base_metrics, _) in tp_diffs)
2184-
sum_diff = sum(int(diff_metrics[col]["Diff executed instructions"]) for (_, _, diff_metrics) in tp_diffs)
2189+
sum_base = sum(int(base_metrics[row]["Diff executed instructions"]) for (_, base_metrics, _) in tp_diffs)
2190+
sum_diff = sum(int(diff_metrics[row]["Diff executed instructions"]) for (_, _, diff_metrics) in tp_diffs)
21852191

2186-
write_fh.write("<summary>{} ({})</summary>\n\n".format(col, format_pct(sum_base, sum_diff)))
2192+
write_fh.write("<summary>{} ({})</summary>\n\n".format(row, format_pct(sum_base, sum_diff)))
21872193
write_fh.write("|Collection|PDIFF|\n")
21882194
write_fh.write("|---|--:|\n")
21892195
for mch_file, base, diff in tp_diffs:
2190-
base_instructions = int(base[col]["Diff executed instructions"])
2191-
diff_instructions = int(diff[col]["Diff executed instructions"])
2196+
base_instructions = int(base[row]["Diff executed instructions"])
2197+
diff_instructions = int(diff[row]["Diff executed instructions"])
21922198

2193-
if significant_diffs[mch_file]:
2199+
if is_significant(row, base, diff):
21942200
write_fh.write("|{}|{}|\n".format(
21952201
mch_file,
21962202
format_pct(base_instructions, diff_instructions)))
@@ -2206,13 +2212,13 @@ def write_pivot_section(col):
22062212

22072213
write_fh.write("\n<details>\n")
22082214
write_fh.write("<summary>Details</summary>\n\n")
2209-
for (disp, col) in [("All", "Overall"), ("MinOpts", "MinOpts"), ("FullOpts", "FullOpts")]:
2215+
for (disp, row) in [("All", "Overall"), ("MinOpts", "MinOpts"), ("FullOpts", "FullOpts")]:
22102216
write_fh.write("{} contexts:\n\n".format(disp))
22112217
write_fh.write("|Collection|Base # instructions|Diff # instructions|PDIFF|\n")
22122218
write_fh.write("|---|--:|--:|--:|\n")
22132219
for mch_file, base, diff in tp_diffs:
2214-
base_instructions = int(base[col]["Diff executed instructions"])
2215-
diff_instructions = int(diff[col]["Diff executed instructions"])
2220+
base_instructions = int(base[row]["Diff executed instructions"])
2221+
diff_instructions = int(diff[row]["Diff executed instructions"])
22162222
write_fh.write("|{}|{:,d}|{:,d}|{}|\n".format(
22172223
mch_file, base_instructions, diff_instructions,
22182224
format_pct(base_instructions, diff_instructions)))

‎src/coreclr/tools/superpmi/superpmi/metricssummary.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ void MetricsSummary::AggregateFrom(const MetricsSummary& other)
1010
SuccessfulCompiles += other.SuccessfulCompiles;
1111
FailingCompiles += other.FailingCompiles;
1212
MissingCompiles += other.MissingCompiles;
13+
NumContextsWithDiffs += other.NumContextsWithDiffs;
1314
NumCodeBytes += other.NumCodeBytes;
1415
NumDiffedCodeBytes += other.NumDiffedCodeBytes;
1516
NumExecutedInstructions += other.NumExecutedInstructions;
@@ -67,7 +68,7 @@ bool MetricsSummaries::SaveToFile(const char* path)
6768

6869
if (!FilePrintf(
6970
file.get(),
70-
"Successful compiles,Failing compiles,Missing compiles,"
71+
"Successful compiles,Failing compiles,Missing compiles,Contexts with diffs,"
7172
"Code bytes,Diffed code bytes,Executed instructions,Diff executed instructions,Name\n"))
7273
{
7374
return false;
@@ -84,10 +85,11 @@ bool MetricsSummaries::WriteRow(HANDLE hFile, const char* name, const MetricsSum
8485
return
8586
FilePrintf(
8687
hFile,
87-
"%d,%d,%d,%lld,%lld,%lld,%lld,%s\n",
88+
"%d,%d,%d,%d,%lld,%lld,%lld,%lld,%s\n",
8889
summary.SuccessfulCompiles,
8990
summary.FailingCompiles,
9091
summary.MissingCompiles,
92+
summary.NumContextsWithDiffs,
9193
summary.NumCodeBytes,
9294
summary.NumDiffedCodeBytes,
9395
summary.NumExecutedInstructions,
@@ -150,17 +152,18 @@ bool MetricsSummaries::LoadFromFile(const char* path, MetricsSummaries* metrics)
150152
int scanResult =
151153
sscanf_s(
152154
line.data(),
153-
"%d,%d,%d,%lld,%lld,%lld,%lld,%s",
155+
"%d,%d,%d,%d,%lld,%lld,%lld,%lld,%s",
154156
&summary.SuccessfulCompiles,
155157
&summary.FailingCompiles,
156158
&summary.MissingCompiles,
159+
&summary.NumContextsWithDiffs,
157160
&summary.NumCodeBytes,
158161
&summary.NumDiffedCodeBytes,
159162
&summary.NumExecutedInstructions,
160163
&summary.NumDiffExecutedInstructions,
161164
name, (unsigned)sizeof(name));
162165

163-
if (scanResult == 8)
166+
if (scanResult == 9)
164167
{
165168
MetricsSummary* tarSummary = nullptr;
166169
if (strcmp(name, "Overall") == 0)

‎src/coreclr/tools/superpmi/superpmi/metricssummary.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ struct MetricsSummary
1212
int FailingCompiles = 0;
1313
// Number of methods that failed jitting due to missing SPMI data.
1414
int MissingCompiles = 0;
15+
// Number of contexts that had any diff.
16+
int NumContextsWithDiffs = 0;
1517
// Number of code bytes produced by the JIT for the successful compiles.
1618
long long NumCodeBytes = 0;
1719
// Number of code bytes that were diffed with the other compiler in diff mode.

‎src/coreclr/tools/superpmi/superpmi/superpmi.cpp‎

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -67,22 +67,22 @@ void SetSuperPmiTargetArchitecture(const char* targetArchitecture)
6767
// This function uses PAL_TRY, so it can't be in the a function that requires object unwinding. Extracting it out here
6868
// avoids compiler error.
6969
//
70-
void InvokeNearDiffer(NearDiffer* nearDiffer,
71-
CommandLine::Options* o,
72-
MethodContext** mc,
73-
CompileResult** crl,
74-
int* matchCount,
75-
MethodContextReader** reader,
76-
MCList* failingMCL,
77-
MCList* diffMCL)
70+
static void InvokeNearDiffer(NearDiffer* nearDiffer,
71+
CommandLine::Options* o,
72+
MethodContext** mc,
73+
CompileResult** crl,
74+
bool* hasDiff,
75+
MethodContextReader** reader,
76+
MCList* failingMCL,
77+
MCList* diffMCL)
7878
{
7979
struct Param : FilterSuperPMIExceptionsParam_CaptureException
8080
{
8181
NearDiffer* nearDiffer;
8282
CommandLine::Options* o;
8383
MethodContext** mc;
8484
CompileResult** crl;
85-
int* matchCount;
85+
bool* hasDiff;
8686
MethodContextReader** reader;
8787
MCList* failingMCL;
8888
MCList* diffMCL;
@@ -91,19 +91,17 @@ void InvokeNearDiffer(NearDiffer* nearDiffer,
9191
param.o = o;
9292
param.mc = mc;
9393
param.crl = crl;
94-
param.matchCount = matchCount;
94+
param.hasDiff = hasDiff;
9595
param.reader = reader;
9696
param.failingMCL = failingMCL;
9797
param.diffMCL = diffMCL;
98+
*hasDiff = false;
9899

99100
PAL_TRY(Param*, pParam, &param)
100101
{
101-
if (pParam->nearDiffer->compare(*pParam->mc, *pParam->crl, (*pParam->mc)->cr))
102-
{
103-
(*pParam->matchCount)++;
104-
}
105-
else
102+
if (!pParam->nearDiffer->compare(*pParam->mc, *pParam->crl, (*pParam->mc)->cr))
106103
{
104+
*pParam->hasDiff = true;
107105
LogIssue(ISSUE_ASM_DIFF, "main method %d of size %d differs", (*pParam->reader)->GetMethodContextIndex(),
108106
(*pParam->mc)->methodSize);
109107

@@ -250,7 +248,6 @@ int __cdecl main(int argc, char* argv[])
250248

251249
int loadedCount = 0;
252250
int jittedCount = 0;
253-
int matchCount = 0;
254251
int failToReplayCount = 0;
255252
int errorCount = 0;
256253
int errorCount2 = 0;
@@ -289,9 +286,9 @@ int __cdecl main(int argc, char* argv[])
289286
st1.Stop();
290287
if (o.applyDiff)
291288
{
292-
LogVerbose(" %2.1f%% - Loaded %d Jitted %d Matching %d FailedCompile %d at %d per second",
293-
reader->PercentComplete(), loadedCount, jittedCount, matchCount, failToReplayCount,
294-
(int)((double)500 / st1.GetSeconds()));
289+
LogVerbose(" %2.1f%% - Loaded %d Jitted %d Diffs %d FailedCompile %d at %d per second",
290+
reader->PercentComplete(), loadedCount, jittedCount, totalBaseMetrics.Overall.NumContextsWithDiffs,
291+
failToReplayCount, (int)((double)500 / st1.GetSeconds()));
295292
}
296293
else
297294
{
@@ -555,7 +552,17 @@ int __cdecl main(int argc, char* argv[])
555552
}
556553
else
557554
{
558-
InvokeNearDiffer(&nearDiffer, &o, &mc, &crl, &matchCount, &reader, &failingToReplayMCL, &diffMCL);
555+
bool hasDiff;
556+
InvokeNearDiffer(&nearDiffer, &o, &mc, &crl, &hasDiff, &reader, &failingToReplayMCL, &diffMCL);
557+
558+
if (hasDiff)
559+
{
560+
totalBaseMetrics.Overall.NumContextsWithDiffs++;
561+
totalDiffMetrics.Overall.NumContextsWithDiffs++;
562+
563+
totalBaseMetricsOpts.NumContextsWithDiffs++;
564+
totalDiffMetricsOpts.NumContextsWithDiffs++;
565+
}
559566

560567
totalBaseMetrics.Overall.NumDiffedCodeBytes += baseMetrics.NumCodeBytes;
561568
totalDiffMetrics.Overall.NumDiffedCodeBytes += diffMetrics.NumCodeBytes;
@@ -638,7 +645,7 @@ int __cdecl main(int argc, char* argv[])
638645
if (o.applyDiff)
639646
{
640647
LogInfo(g_AsmDiffsSummaryFormatString, loadedCount, jittedCount, failToReplayCount, excludedCount,
641-
missingCount, jittedCount - failToReplayCount - matchCount);
648+
missingCount, totalDiffMetrics.Overall.NumContextsWithDiffs);
642649
}
643650
else
644651
{
@@ -679,7 +686,7 @@ int __cdecl main(int argc, char* argv[])
679686
{
680687
result = SpmiResult::Error;
681688
}
682-
else if (o.applyDiff && (matchCount != jittedCount - missingCount))
689+
else if (o.applyDiff && (totalDiffMetrics.Overall.NumContextsWithDiffs > 0))
683690
{
684691
result = SpmiResult::Diffs;
685692
}

0 commit comments

Comments
 (0)