From ba7bd0c5498017151401a5c0b02afe7224c9aeb5 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 25 Aug 2022 16:51:47 +0200 Subject: [PATCH] JIT: Enhance metrics reported by superpmi diffs * Report the total number of contexts, minopts contexts and fullopts contexted processed * Report number of successful and missing contexts * Report asmdiffs and tpdiffs for minopts/fullopts separately Fixes #70350 Contributes to #73506 --- src/coreclr/scripts/superpmi.py | 200 +++++++++++++----- .../scripts/superpmi_diffs_summarize.py | 35 +-- .../superpmi-shared/methodcontext.cpp | 1 + .../tools/superpmi/superpmi/jitinstance.cpp | 6 +- .../tools/superpmi/superpmi/jitinstance.h | 2 +- .../superpmi/superpmi/metricssummary.cpp | 173 ++++++++++----- .../tools/superpmi/superpmi/metricssummary.h | 20 +- .../superpmi/superpmi/parallelsuperpmi.cpp | 24 ++- .../tools/superpmi/superpmi/superpmi.cpp | 53 +++-- 9 files changed, 349 insertions(+), 165 deletions(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 9e30bf5e77cfb..cf15911a984ab 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -478,21 +478,25 @@ def create_artifacts_base_name(coreclr_args, mch_file): def read_csv_metrics(path): - """ Read a metrics summary file produced by superpmi, and return the single row containing the information as a dictionary. + """ Read a metrics summary file produced by superpmi and return the rows as a dictionary of dictionaries. Args: path (str) : path to .csv file Returns: - A dictionary with each metric + A dictionary of dictionaries. For example, dict["Total"]["Successful + compiles"] will access the total number of successful compiles and + dict["MinOpts"]["Successful compiles"] will access the number of + minopts compilations. """ + dict = {} with open(path) as csv_file: reader = csv.DictReader(csv_file) for row in reader: - return row + dict[row["Name"]] = row - return None + return dict def determine_clrjit_compiler_version(clrjit_path): """ Obtain the version of the compiler that was used to compile the clrjit at the specified path. @@ -1188,7 +1192,7 @@ def print_superpmi_result(return_code, coreclr_args, base_metrics, diff_metrics) those return codes. """ if return_code == 0: - logging.info("Clean SuperPMI {} ({} contexts processed)".format("replay" if diff_metrics is None else "diff", base_metrics["Successful compiles"])) + logging.info("Clean SuperPMI {} ({} contexts processed)".format("replay" if diff_metrics is None else "diff", base_metrics["Total"]["Successful compiles"])) elif return_code == -1 or return_code == 4294967295: logging.error("General fatal error") elif return_code == -2 or return_code == 4294967294: @@ -1198,13 +1202,13 @@ def print_superpmi_result(return_code, coreclr_args, base_metrics, diff_metrics) elif return_code == 2: logging.warning("Asm diffs found") elif return_code == 3: - missing_base = int(base_metrics["Missing compiles"]) - total_contexts = int(base_metrics["Successful compiles"]) + int(base_metrics["Failing compiles"]) + missing_base = int(base_metrics["Total"]["Missing compiles"]) + total_contexts = int(base_metrics["Total"]["Successful compiles"]) + int(base_metrics["Total"]["Failing compiles"]) if diff_metrics is None: logging.warning("SuperPMI encountered missing data for {} out of {} contexts".format(missing_base, total_contexts)) else: - missing_diff = int(diff_metrics["Missing compiles"]) + missing_diff = int(diff_metrics["Total"]["Missing compiles"]) logging.warning("SuperPMI encountered missing data. Missing with base JIT: {}. Missing with diff JIT: {}. Total contexts: {}.".format(missing_base, missing_diff, total_contexts)) elif return_code == 139 and coreclr_args.host_os != "windows": @@ -1521,7 +1525,7 @@ def replay_with_asm_diffs(self): files_with_replay_failures = [] # List of all Markdown summary files - all_md_summary_files = [] + asm_diffs = [] with TempDir(None, self.coreclr_args.skip_cleanup) as temp_location: logging.debug("") @@ -1606,13 +1610,16 @@ def replay_with_asm_diffs(self): save_repro_mc_files(temp_location, self.coreclr_args, artifacts_base_name, repro_base_command_line) # This file had asm diffs; keep track of that. - if is_nonzero_length_file(diff_mcl_file): + has_diffs = is_nonzero_length_file(diff_mcl_file) + if has_diffs: files_with_asm_diffs.append(mch_file) # There were diffs. Go through each method that created diffs and - # create a base/diff asm file with diffable asm. In addition, create - # a standalone .mc for easy iteration. - if is_nonzero_length_file(diff_mcl_file) and not self.coreclr_args.diff_with_release: + # create a base/diff asm file with diffable asm so we can analyze + # it. In addition, create a standalone .mc for easy iteration. + jit_analyze_summary_file = None + + if has_diffs and not self.coreclr_args.diff_with_release: # AsmDiffs. Save the contents of the fail.mcl file to dig into failures. if return_code == 0: @@ -1711,8 +1718,8 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: logging.debug("") if base_metrics is not None and diff_metrics is not None: - base_bytes = int(base_metrics["Diffed code bytes"]) - diff_bytes = int(diff_metrics["Diffed code bytes"]) + base_bytes = int(base_metrics["Total"]["Diffed code bytes"]) + diff_bytes = int(diff_metrics["Total"]["Diffed code bytes"]) logging.info("Total bytes of base: {}".format(base_bytes)) logging.info("Total bytes of diff: {}".format(diff_bytes)) delta_bytes = diff_bytes - base_bytes @@ -1736,10 +1743,8 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: jit_analyze_path = find_file(jit_analyze_file, path_var.split(os.pathsep)) if jit_analyze_path is not None: # It appears we have a built jit-analyze on the path, so try to run it. - md_summary_file = os.path.join(asm_root_dir, "summary.md") - summary_file_info = ( mch_file, md_summary_file ) - all_md_summary_files.append(summary_file_info) - command = [ jit_analyze_path, "--md", md_summary_file, "-r", "--base", base_asm_location, "--diff", diff_asm_location ] + jit_analyze_summary_file = os.path.join(asm_root_dir, "summary.md") + command = [ jit_analyze_path, "--md", jit_analyze_summary_file, "-r", "--base", base_asm_location, "--diff", diff_asm_location ] if self.coreclr_args.retainOnlyTopFiles: command += [ "--retainOnlyTopFiles" ] if self.coreclr_args.metrics: @@ -1785,15 +1790,18 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: logging.warning("No textual differences found in generated JitDump. Is this an issue with coredistools?") if base_metrics is not None and diff_metrics is not None: - missing_base = int(base_metrics["Missing compiles"]) - missing_diff = int(diff_metrics["Missing compiles"]) - total_contexts = int(base_metrics["Successful compiles"]) + int(base_metrics["Failing compiles"]) + missing_base = int(base_metrics["Total"]["Missing compiles"]) + missing_diff = int(diff_metrics["Total"]["Missing compiles"]) + total_contexts = int(base_metrics["Total"]["Successful compiles"]) + int(base_metrics["Total"]["Failing compiles"]) if missing_base > 0 or missing_diff > 0: logging.warning("Warning: SuperPMI encountered missing data during the diff. The diff summary printed above may be misleading.") logging.warning("Missing with base JIT: {}. Missing with diff JIT: {}. Total contexts: {}.".format(missing_base, missing_diff, total_contexts)) - ################################################################################################ end of processing asm diffs (if is_nonzero_length_file(diff_mcl_file)... + ################################################################################################ end of processing asm diffs (if has_diffs... + + mch_file_basename = os.path.basename(mch_file) + asm_diffs.append((mch_file_basename, base_metrics, diff_metrics, has_diffs, jit_analyze_summary_file)) if not self.coreclr_args.skip_cleanup: if os.path.isfile(fail_mcl_file): @@ -1816,7 +1824,7 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: # Construct an overall Markdown summary file. - if len(all_md_summary_files) > 0 and not self.coreclr_args.diff_with_release: + if len(asm_diffs) > 0 and not self.coreclr_args.diff_with_release: overall_md_summary_file = create_unique_file_name(self.coreclr_args.spmi_location, "diff_summary", "md") if not os.path.isdir(self.coreclr_args.spmi_location): os.makedirs(self.coreclr_args.spmi_location) @@ -1824,13 +1832,89 @@ async def create_one_artifact(jit_path: str, location: str, flags) -> str: os.remove(overall_md_summary_file) with open(overall_md_summary_file, "w") as write_fh: - for summary_file_info in all_md_summary_files: - summary_mch = summary_file_info[0] - summary_mch_filename = os.path.basename(summary_mch) # Display just the MCH filename, not the full path - summary_file = summary_file_info[1] - with open(summary_file, "r") as read_fh: - write_fh.write("## " + summary_mch_filename + ":\n\n") - shutil.copyfileobj(read_fh, write_fh) + def format_delta(base, diff): + plus_if_positive = "+" if diff >= base else "" + pct = "" + if base > 0: + pct = " ({}{:.2f}%)".format(plus_if_positive, (diff - base) / base * 100) + + return "{}{:,d} B{}".format(plus_if_positive, diff - base, pct) + + diffed_contexts = sum(int(diff_metrics["Total"]["Successful compiles"]) for (_, _, diff_metrics, _, _) in asm_diffs) + diffed_minopts_contexts = sum(int(diff_metrics["MinOpts"]["Successful compiles"]) for (_, _, diff_metrics, _, _) in asm_diffs) + diffed_opts_contexts = sum(int(diff_metrics["Opts"]["Successful compiles"]) for (_, _, diff_metrics, _, _) in asm_diffs) + missing_base_contexts = sum(int(base_metrics["Total"]["Missing compiles"]) for (_, base_metrics, _, _, _) in asm_diffs) + missing_diff_contexts = sum(int(diff_metrics["Total"]["Missing compiles"]) for (_, _, diff_metrics, _, _) in asm_diffs) + + write_fh.write("Diffs are based on {:,d} contexts ({:,d} minopts, {:,d} optimized).\n".format( + diffed_contexts, diffed_minopts_contexts, diffed_opts_contexts)) + + write_fh.write("{:,d} contexts missed with the base JIT and {:,d} missed with the diff JIT.\n\n".format(missing_base_contexts, missing_diff_contexts)) + + if any(has_diff for (_, _, _, has_diff, _) in asm_diffs): + # First write an overview table + write_fh.write("|Collection|Overall|MinOpts|Opts|\n") + write_fh.write("|---|---|---|---|\n") + for (mch_file, base_metrics, diff_metrics, has_diffs, jit_analyze_summary_file) in asm_diffs: + if not has_diffs: + continue + + write_fh.write("|{}|{}|{}|{}|\n".format( + mch_file, + format_delta( + int(base_metrics["Total"]["Diffed code bytes"]), + int(diff_metrics["Total"]["Diffed code bytes"])), + format_delta( + int(base_metrics["MinOpts"]["Diffed code bytes"]), + int(diff_metrics["MinOpts"]["Diffed code bytes"])), + format_delta( + int(base_metrics["Opts"]["Diffed code bytes"]), + int(diff_metrics["Opts"]["Diffed code bytes"])))) + + write_fh.write("\n") + else: + write_fh.write("No diffs found.\n") + + # Next write a detailed section + write_fh.write("\n
\n") + write_fh.write("Details\n\n") + + write_fh.write("|Collection|Diffed contexts|Base missing contexts|Diff missing contexts|Minopts contexts|Optimized contexts|\n") + write_fh.write("|---|---|---|---|---|---|\n") + for (mch_file, base_metrics, diff_metrics, has_diffs, jit_analyze_summary_file) in asm_diffs: + write_fh.write("|{}|{:,d}|{:,d}|{:,d}|{:,d}|{:,d}|\n".format( + mch_file, + int(diff_metrics["Total"]["Successful compiles"]), + int(base_metrics["Total"]["Missing compiles"]), + int(diff_metrics["Total"]["Missing compiles"]), + int(diff_metrics["MinOpts"]["Successful compiles"]), + int(diff_metrics["Opts"]["Successful compiles"]))) + + write_fh.write("\n") + + if any(has_diff for (_, _, _, has_diff, _) in asm_diffs): + for (mch_file, base_metrics, diff_metrics, has_diffs, jit_analyze_summary_file) in asm_diffs: + if not has_diffs or jit_analyze_summary_file is None: + continue + + with open(jit_analyze_summary_file, "r") as read_fh: + write_fh.write("""\ + +
+ +{0} + +To reproduce these diffs on Windows {1}: +``` +superpmi.py asmdiffs -target_os {2} -target_arch {3} -arch {1} +``` + +""".format(mch_file, self.coreclr_args.arch, self.coreclr_args.target_os, self.coreclr_args.target_arch)) + + shutil.copyfileobj(read_fh, write_fh) + write_fh.write("\n
\n") + + write_fh.write("\n
\n") logging.info(" Summary Markdown file: %s", overall_md_summary_file) @@ -1977,15 +2061,15 @@ def replay_with_throughput_diff(self): diff_metrics = read_csv_metrics(diff_metrics_summary_file) if base_metrics is not None and diff_metrics is not None: - base_instructions = int(base_metrics["Diff executed instructions"]) - diff_instructions = int(diff_metrics["Diff executed instructions"]) + base_instructions = int(base_metrics["Total"]["Diff executed instructions"]) + diff_instructions = int(diff_metrics["Total"]["Diff executed instructions"]) logging.info("Total instructions executed by base: {}".format(base_instructions)) logging.info("Total instructions executed by diff: {}".format(diff_instructions)) if base_instructions != 0 and diff_instructions != 0: delta_instructions = diff_instructions - base_instructions logging.info("Total instructions executed delta: {} ({:.2%} of base)".format(delta_instructions, delta_instructions / base_instructions)) - tp_diffs.append((os.path.basename(mch_file), base_instructions, diff_instructions)) + tp_diffs.append((os.path.basename(mch_file), base_metrics, diff_metrics)) else: logging.warning("One compilation failed to produce any results") else: @@ -2024,30 +2108,48 @@ def replay_with_throughput_diff(self): # We write two tables, an overview one with just significantly # impacted collections and a detailed one that includes raw # instruction count and all collections. - def is_significant(base, diff): + def is_significant_pct(base, diff): return round((diff - base) / base * 100, 2) != 0 + def is_significant(base, diff): + def check(col): + return is_significant_pct(int(base[col]["Diff executed instructions"]), int(diff[col]["Diff executed instructions"])) + + return check("Total") or check("MinOpts") or check("Opts") + def format_pct(base_instructions, diff_instructions): + plus_if_positive = "+" if diff_instructions > base_instructions else "" + return "{}{:.2f}%".format(plus_if_positive, (diff_instructions - base_instructions) / base_instructions * 100) if any(is_significant(base, diff) for (_, base, diff) in tp_diffs): - write_fh.write("|Collection|PDIFF|\n") - write_fh.write("|---|---|\n") - for mch_file, base_instructions, diff_instructions in tp_diffs: - if is_significant(base_instructions, diff_instructions): - write_fh.write("|{}|{}{:.2f}%|\n".format( + write_fh.write("|Collection|Overall|MinOpts|Opts|\n") + write_fh.write("|---|---|---|---|\n") + for mch_file, base, diff in tp_diffs: + def format_pct_for_col(col): + base_instructions = int(base[col]["Diff executed instructions"]) + diff_instructions = int(diff[col]["Diff executed instructions"]) + return format_pct(base_instructions, diff_instructions) + + if is_significant(base, diff): + write_fh.write("|{}|{}|{}|{}|\n".format( mch_file, - "+" if diff_instructions > base_instructions else "", - (diff_instructions - base_instructions) / base_instructions * 100)) + format_pct_for_col("Total"), + format_pct_for_col("MinOpts"), + format_pct_for_col("Opts"))) else: write_fh.write("No significant throughput differences found\n") write_fh.write("\n
\n") write_fh.write("Details\n\n") - write_fh.write("|Collection|Base # instructions|Diff # instructions|PDIFF|\n") - write_fh.write("|---|---|---|---|\n") - for mch_file, base_instructions, diff_instructions in tp_diffs: - write_fh.write("|{}|{:,d}|{:,d}|{}{:.2f}%|\n".format( - mch_file, base_instructions, diff_instructions, - "+" if diff_instructions > base_instructions else "", - (diff_instructions - base_instructions) / base_instructions * 100)) + for (disp, col) in [("All", "Total"), ("MinOpts", "MinOpts"), ("Optimized", "Opts")]: + write_fh.write("{} contexts:\n\n".format(disp)) + write_fh.write("|Collection|Base # instructions|Diff # instructions|PDIFF|\n") + write_fh.write("|---|---|---|---|\n") + for mch_file, base, diff in tp_diffs: + base_instructions = int(base[col]["Diff executed instructions"]) + diff_instructions = int(diff[col]["Diff executed instructions"]) + write_fh.write("|{}|{:,d}|{:,d}|{}|\n".format( + mch_file, base_instructions, diff_instructions, + format_pct(base_instructions, diff_instructions))) + write_fh.write("\n") write_fh.write("\n
\n") logging.info(" Summary Markdown file: %s", overall_md_summary_file) diff --git a/src/coreclr/scripts/superpmi_diffs_summarize.py b/src/coreclr/scripts/superpmi_diffs_summarize.py index 95f3a2cb389f0..a75d01bd8229e 100644 --- a/src/coreclr/scripts/superpmi_diffs_summarize.py +++ b/src/coreclr/scripts/superpmi_diffs_summarize.py @@ -89,39 +89,8 @@ def append_diff_file(f, arch, file_name, full_file_path, asmdiffs): else: diffs_found = True f.write("## {} {}\n".format(diff_os, diff_arch)) - - if asmdiffs: - # Contents of asmdiffs are large so create a - #
...
disclosure section around - # the file and additionally provide some instructions. - f.write("""\ - -
- -{0} {1} details - -Summary file: `{2}` - -To reproduce these diffs on Windows {3}: -``` -superpmi.py asmdiffs -target_os {0} -target_arch {1} -arch {3} -``` - -""".format(diff_os, diff_arch, file_name, arch)) - - # Now write the contents - f.write(contents) - - # Write the footer (close the
section) - f.write("""\ - -
- -""") - - else: - f.write(contents) - f.write("\n") + f.write(contents) + f.write("\n") return diffs_found diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index af97ac298c939..e876dbf4e5dfd 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -1252,6 +1252,7 @@ DWORD MethodContext::repGetJitFlags(CORJIT_FLAGS* jitFlags, DWORD sizeInBytes) DEBUG_REP(dmpGetJitFlags(0, value)); CORJIT_FLAGS* resultFlags = (CORJIT_FLAGS*)GetJitFlags->GetBuffer(value.A); + Assert(sizeInBytes >= value.B); memcpy(jitFlags, resultFlags, value.B); InitReadyToRunFlag(resultFlags); return value.B; diff --git a/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp b/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp index da48141381837..b96fef776fde5 100644 --- a/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp +++ b/src/coreclr/tools/superpmi/superpmi/jitinstance.cpp @@ -298,7 +298,7 @@ extern "C" DLLEXPORT NOINLINE void Instrumentor_GetInsCount(UINT64* result) } } -JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput, MetricsSummary* metrics) +JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput, MetricsSummary* metrics, CORJIT_FLAGS* jitFlags) { struct Param : FilterSuperPMIExceptionsParam_CaptureException { @@ -309,6 +309,7 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i int mcIndex; bool collectThroughput; MetricsSummary* metrics; + CORJIT_FLAGS* jitFlags; } param; param.pThis = this; param.result = RESULT_SUCCESS; // assume success @@ -316,6 +317,7 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i param.mcIndex = mcIndex; param.collectThroughput = collectThroughput; param.metrics = metrics; + param.jitFlags = jitFlags; // store to instance field our raw values, so we can figure things out a bit later... mc = MethodToCompile; @@ -335,6 +337,7 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i CORINFO_OS os = CORINFO_WINNT; pParam->pThis->mc->repCompileMethod(&pParam->info, &pParam->flags, &os); + pParam->pThis->mc->repGetJitFlags(pParam->jitFlags, sizeof(*pParam->jitFlags)); if (pParam->collectThroughput) { pParam->pThis->lt.Start(); @@ -436,7 +439,6 @@ JitInstance::Result JitInstance::CompileMethod(MethodContext* MethodToCompile, i { metrics->SuccessfulCompiles++; metrics->NumExecutedInstructions += static_cast(insCountAfter - insCountBefore); - } else { diff --git a/src/coreclr/tools/superpmi/superpmi/jitinstance.h b/src/coreclr/tools/superpmi/superpmi/jitinstance.h index 8e8fbabf82ead..7ff4bd4a7004b 100644 --- a/src/coreclr/tools/superpmi/superpmi/jitinstance.h +++ b/src/coreclr/tools/superpmi/superpmi/jitinstance.h @@ -60,7 +60,7 @@ class JitInstance bool resetConfig(MethodContext* firstContext); - Result CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput, class MetricsSummary* summary); + Result CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput, struct MetricsSummary* metrics, class CORJIT_FLAGS* jitFlags); const WCHAR* getForceOption(const WCHAR* key); const WCHAR* getOption(const WCHAR* key); diff --git a/src/coreclr/tools/superpmi/superpmi/metricssummary.cpp b/src/coreclr/tools/superpmi/superpmi/metricssummary.cpp index cf93ce902dedd..64d9c5216c108 100644 --- a/src/coreclr/tools/superpmi/superpmi/metricssummary.cpp +++ b/src/coreclr/tools/superpmi/superpmi/metricssummary.cpp @@ -5,13 +5,23 @@ #include "metricssummary.h" #include "logging.h" -struct HandleCloser +void MetricsSummary::AggregateFrom(const MetricsSummary& other) { - void operator()(HANDLE hFile) - { - CloseHandle(hFile); - } -}; + SuccessfulCompiles += other.SuccessfulCompiles; + FailingCompiles += other.FailingCompiles; + MissingCompiles += other.MissingCompiles; + NumCodeBytes += other.NumCodeBytes; + NumDiffedCodeBytes += other.NumDiffedCodeBytes; + NumExecutedInstructions += other.NumExecutedInstructions; + NumDiffExecutedInstructions += other.NumDiffExecutedInstructions; +} + +void MetricsSummaries::AggregateFrom(const MetricsSummaries& other) +{ + Total.AggregateFrom(other.Total); + MinOpts.AggregateFrom(other.MinOpts); + Opts.AggregateFrom(other.Opts); +} struct FileHandleWrapper { @@ -31,7 +41,23 @@ struct FileHandleWrapper HANDLE hFile; }; -bool MetricsSummary::SaveToFile(const char* path) +static bool FilePrintf(HANDLE hFile, const char* fmt, ...) +{ + va_list args; + va_start(args, fmt); + + char buffer[4096]; + int len = vsprintf_s(buffer, fmt, args); + DWORD numWritten; + bool result = + WriteFile(hFile, buffer, static_cast(len), &numWritten, nullptr) && (numWritten == static_cast(len)); + + va_end(args); + + return result; +} + +bool MetricsSummaries::SaveToFile(const char* path) { FileHandleWrapper file(CreateFile(path, GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr)); if (file.get() == INVALID_HANDLE_VALUE) @@ -39,29 +65,37 @@ bool MetricsSummary::SaveToFile(const char* path) return false; } - char buffer[4096]; - int len = - sprintf_s( - buffer, sizeof(buffer), - "Successful compiles,Failing compiles,Missing compiles,Code bytes,Diffed code bytes,Executed instructions,Diff executed instructions\n" - "%d,%d,%d,%lld,%lld,%lld,%lld\n", - SuccessfulCompiles, - FailingCompiles, - MissingCompiles, - NumCodeBytes, - NumDiffedCodeBytes, - NumExecutedInstructions, - NumDiffExecutedInstructions); - DWORD numWritten; - if (!WriteFile(file.get(), buffer, static_cast(len), &numWritten, nullptr) || numWritten != static_cast(len)) + if (!FilePrintf( + file.get(), + "Successful compiles,Failing compiles,Missing compiles," + "Code bytes,Diffed code bytes,Executed instructions,Diff executed instructions,Name\n")) { return false; } - return true; + return + WriteRow(file.get(), "Total", Total) && + WriteRow(file.get(), "MinOpts", MinOpts) && + WriteRow(file.get(), "Opts", Opts); +} + +bool MetricsSummaries::WriteRow(HANDLE hFile, const char* name, const MetricsSummary& summary) +{ + return + FilePrintf( + hFile, + "%d,%d,%d,%lld,%lld,%lld,%lld,%s\n", + summary.SuccessfulCompiles, + summary.FailingCompiles, + summary.MissingCompiles, + summary.NumCodeBytes, + summary.NumDiffedCodeBytes, + summary.NumExecutedInstructions, + summary.NumDiffExecutedInstructions, + name); } -bool MetricsSummary::LoadFromFile(const char* path, MetricsSummary* metrics) +bool MetricsSummaries::LoadFromFile(const char* path, MetricsSummaries* metrics) { FileHandleWrapper file(CreateFile(path, GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)); if (file.get() == INVALID_HANDLE_VALUE) @@ -76,38 +110,73 @@ bool MetricsSummary::LoadFromFile(const char* path, MetricsSummary* metrics) } DWORD stringLen = static_cast(len.QuadPart); - std::vector content(stringLen + 1); + std::vector content(stringLen); DWORD numRead; - if (!ReadFile(file.get(), content.data(), stringLen, &numRead, nullptr) || numRead != stringLen) + if (!ReadFile(file.get(), content.data(), stringLen, &numRead, nullptr) || (numRead != stringLen)) { return false; } - content[stringLen] = '\0'; - - int scanResult = - sscanf_s( - content.data(), - "Successful compiles,Failing compiles,Missing compiles,Code bytes,Diffed code bytes,Executed instructions,Diff executed instructions\n" - "%d,%d,%d,%lld,%lld,%lld,%lld\n", - &metrics->SuccessfulCompiles, - &metrics->FailingCompiles, - &metrics->MissingCompiles, - &metrics->NumCodeBytes, - &metrics->NumDiffedCodeBytes, - &metrics->NumExecutedInstructions, - &metrics->NumDiffExecutedInstructions); - - return scanResult == 7; -} + std::vector line; + size_t index = 0; + auto nextLine = [&line, &content, &index]() + { + size_t end = index; + while ((end < content.size()) && (content[end] != '\r') && (content[end] != '\n')) + { + end++; + } -void MetricsSummary::AggregateFrom(const MetricsSummary& other) -{ - SuccessfulCompiles += other.SuccessfulCompiles; - FailingCompiles += other.FailingCompiles; - MissingCompiles += other.MissingCompiles; - NumCodeBytes += other.NumCodeBytes; - NumDiffedCodeBytes += other.NumDiffedCodeBytes; - NumExecutedInstructions += other.NumExecutedInstructions; - NumDiffExecutedInstructions += other.NumDiffExecutedInstructions; + line.resize(end - index + 1); + memcpy(line.data(), &content[index], end - index); + line[end - index] = '\0'; + + index = end; + if ((index < content.size()) && (content[index] == '\r')) + index++; + if ((index < content.size()) && (content[index] == '\n')) + index++; + }; + + *metrics = MetricsSummaries(); + nextLine(); + bool result = true; + while (index < content.size()) + { + nextLine(); + MetricsSummary summary; + + char name[32]; + int scanResult = + sscanf_s( + line.data(), + "%d,%d,%d,%lld,%lld,%lld,%lld,%s", + &summary.SuccessfulCompiles, + &summary.FailingCompiles, + &summary.MissingCompiles, + &summary.NumCodeBytes, + &summary.NumDiffedCodeBytes, + &summary.NumExecutedInstructions, + &summary.NumDiffExecutedInstructions, + name, (unsigned)sizeof(name)); + + if (scanResult == 8) + { + MetricsSummary* tarSummary = nullptr; + if (strcmp(name, "Total") == 0) + metrics->Total = summary; + else if (strcmp(name, "MinOpts") == 0) + metrics->MinOpts = summary; + else if (strcmp(name, "Opts") == 0) + metrics->Opts = summary; + else + result = false; + } + else + { + result = false; + } + } + + return result; } diff --git a/src/coreclr/tools/superpmi/superpmi/metricssummary.h b/src/coreclr/tools/superpmi/superpmi/metricssummary.h index 5120835575520..03bff16a94db6 100644 --- a/src/coreclr/tools/superpmi/superpmi/metricssummary.h +++ b/src/coreclr/tools/superpmi/superpmi/metricssummary.h @@ -4,9 +4,8 @@ #ifndef _MetricsSummary #define _MetricsSummary -class MetricsSummary +struct MetricsSummary { -public: // Number of methods successfully jitted. int SuccessfulCompiles = 0; // Number of methods that failed jitting. @@ -23,9 +22,22 @@ class MetricsSummary // Number of executed instructions inside contexts that were successfully diffed. long long NumDiffExecutedInstructions = 0; - bool SaveToFile(const char* path); - static bool LoadFromFile(const char* path, MetricsSummary* metrics); void AggregateFrom(const MetricsSummary& other); }; +class MetricsSummaries +{ +public: + MetricsSummary Total; + MetricsSummary MinOpts; + MetricsSummary Opts; + + void AggregateFrom(const MetricsSummaries& other); + + bool SaveToFile(const char* path); + static bool LoadFromFile(const char* path, MetricsSummaries* metrics); +private: + static bool WriteRow(HANDLE hFile, const char* name, const MetricsSummary& summary); +}; + #endif diff --git a/src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp b/src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp index 92b172e196d1c..dfca5adcc376a 100644 --- a/src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp +++ b/src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp @@ -294,12 +294,16 @@ void ProcessChildStdOut(const CommandLine::Options& o, } } -static bool ProcessChildMetrics(const char* baseMetricsSummaryPath, const char* diffMetricsSummaryPath, MetricsSummary* baseMetrics, MetricsSummary* diffMetrics) +static bool ProcessChildMetrics( + const char* baseMetricsSummaryPath, + MetricsSummaries* baseMetrics, + const char* diffMetricsSummaryPath, + MetricsSummaries* diffMetrics) { if (baseMetricsSummaryPath != nullptr) { - MetricsSummary childBaseMetrics; - if (!MetricsSummary::LoadFromFile(baseMetricsSummaryPath, &childBaseMetrics)) + MetricsSummaries childBaseMetrics; + if (!MetricsSummaries::LoadFromFile(baseMetricsSummaryPath, &childBaseMetrics)) { LogError("Couldn't load base metrics summary created by child process"); return false; @@ -310,8 +314,8 @@ static bool ProcessChildMetrics(const char* baseMetricsSummaryPath, const char* if (diffMetricsSummaryPath != nullptr) { - MetricsSummary childDiffMetrics; - if (!MetricsSummary::LoadFromFile(diffMetricsSummaryPath, &childDiffMetrics)) + MetricsSummaries childDiffMetrics; + if (!MetricsSummaries::LoadFromFile(diffMetricsSummaryPath, &childDiffMetrics)) { LogError("Couldn't load diff metrics summary created by child process"); return false; @@ -556,13 +560,13 @@ int doParallelSuperPMI(CommandLine::Options& o) if (o.baseMetricsSummaryFile != nullptr) { wd.baseMetricsSummaryPath = new char[MAX_PATH]; - sprintf_s(wd.baseMetricsSummaryPath, MAX_PATH, "%sParallelSuperPMI-BaseMetricsSummary-%u-%d.txt", tempPath, randNumber, i); + sprintf_s(wd.baseMetricsSummaryPath, MAX_PATH, "%sParallelSuperPMI-BaseMetricsSummary-%u-%d.csv", tempPath, randNumber, i); } if (o.diffMetricsSummaryFile != nullptr) { wd.diffMetricsSummaryPath = new char[MAX_PATH]; - sprintf_s(wd.diffMetricsSummaryPath, MAX_PATH, "%sParallelSuperPMI-DiffMetricsSummary-%u-%d.txt", tempPath, randNumber, i); + sprintf_s(wd.diffMetricsSummaryPath, MAX_PATH, "%sParallelSuperPMI-DiffMetricsSummary-%u-%d.csv", tempPath, randNumber, i); } wd.stdOutputPath = new char[MAX_PATH]; @@ -693,8 +697,8 @@ int doParallelSuperPMI(CommandLine::Options& o) bool usageError = false; // variable to flag if we hit a usage error in SuperPMI int loaded = 0, jitted = 0, failed = 0, excluded = 0, missing = 0, diffs = 0; - MetricsSummary baseMetrics; - MetricsSummary diffMetrics; + MetricsSummaries baseMetrics; + MetricsSummaries diffMetrics; // Read the stderr files and log them as errors // Read the stdout files and parse them for counts and log any MISSING or ISSUE errors @@ -703,7 +707,7 @@ int doParallelSuperPMI(CommandLine::Options& o) PerWorkerData& wd = perWorkerData[i]; ProcessChildStdErr(wd.stdErrorPath); ProcessChildStdOut(o, wd.stdOutputPath, &loaded, &jitted, &failed, &excluded, &missing, &diffs, &usageError); - ProcessChildMetrics(wd.baseMetricsSummaryPath, wd.diffMetricsSummaryPath, &baseMetrics, &diffMetrics); + ProcessChildMetrics(wd.baseMetricsSummaryPath, &baseMetrics, wd.diffMetricsSummaryPath, &diffMetrics); if (usageError) break; diff --git a/src/coreclr/tools/superpmi/superpmi/superpmi.cpp b/src/coreclr/tools/superpmi/superpmi/superpmi.cpp index c43ed6fad684d..9740ffc3ed73c 100644 --- a/src/coreclr/tools/superpmi/superpmi/superpmi.cpp +++ b/src/coreclr/tools/superpmi/superpmi/superpmi.cpp @@ -269,8 +269,8 @@ int __cdecl main(int argc, char* argv[]) } } - MetricsSummary totalBaseMetrics; - MetricsSummary totalDiffMetrics; + MetricsSummaries totalBaseMetrics; + MetricsSummaries totalDiffMetrics; while (true) { @@ -367,18 +367,32 @@ int __cdecl main(int argc, char* argv[]) } MetricsSummary baseMetrics; + CORJIT_FLAGS jitFlags; jittedCount++; st3.Start(); - res = jit->CompileMethod(mc, reader->GetMethodContextIndex(), collectThroughput, &baseMetrics); + res = jit->CompileMethod(mc, reader->GetMethodContextIndex(), collectThroughput, &baseMetrics, &jitFlags); st3.Stop(); LogDebug("Method %d compiled%s in %fms, result %d", reader->GetMethodContextIndex(), (o.nameOfJit2 == nullptr) ? "" : " by JIT1", st3.GetMilliseconds(), res); - totalBaseMetrics.AggregateFrom(baseMetrics); + bool isMinOpts = + jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_DEBUG_CODE) || + jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_MIN_OPT) || + jitFlags.IsSet(CORJIT_FLAGS::CORJIT_FLAG_TIER0); - if ((res == JitInstance::RESULT_SUCCESS) && Logger::IsLogLevelEnabled(LOGLEVEL_DEBUG)) + MetricsSummary& totalBaseMetricsOpts = isMinOpts ? totalBaseMetrics.MinOpts : totalBaseMetrics.Opts; + MetricsSummary& totalDiffMetricsOpts = isMinOpts ? totalDiffMetrics.MinOpts : totalDiffMetrics.Opts; + + totalBaseMetrics.Total.AggregateFrom(baseMetrics); + + if (res == JitInstance::RESULT_SUCCESS) { - mc->cr->dumpToConsole(); // Dump the compile results if doing debug logging + totalBaseMetricsOpts.AggregateFrom(baseMetrics); + + if (Logger::IsLogLevelEnabled(LOGLEVEL_DEBUG)) + { + mc->cr->dumpToConsole(); // Dump the compile results if doing debug logging + } } MetricsSummary diffMetrics; @@ -392,16 +406,21 @@ int __cdecl main(int argc, char* argv[]) mc->cr = new CompileResult(); st4.Start(); - res2 = jit2->CompileMethod(mc, reader->GetMethodContextIndex(), collectThroughput, &diffMetrics); + res2 = jit2->CompileMethod(mc, reader->GetMethodContextIndex(), collectThroughput, &diffMetrics, &jitFlags); st4.Stop(); LogDebug("Method %d compiled by JIT2 in %fms, result %d", reader->GetMethodContextIndex(), st4.GetMilliseconds(), res2); - totalDiffMetrics.AggregateFrom(diffMetrics); + totalDiffMetrics.Total.AggregateFrom(diffMetrics); - if ((res2 == JitInstance::RESULT_SUCCESS) && Logger::IsLogLevelEnabled(LOGLEVEL_DEBUG)) + if (res2 == JitInstance::RESULT_SUCCESS) { - mc->cr->dumpToConsole(); // Dump the compile results if doing debug logging + totalDiffMetricsOpts.AggregateFrom(diffMetrics); + + if (Logger::IsLogLevelEnabled(LOGLEVEL_DEBUG)) + { + mc->cr->dumpToConsole(); // Dump the compile results if doing debug logging + } } if (res2 == JitInstance::RESULT_ERROR) @@ -542,11 +561,17 @@ int __cdecl main(int argc, char* argv[]) { InvokeNearDiffer(&nearDiffer, &o, &mc, &crl, &matchCount, &reader, &failingToReplayMCL, &diffMCL); - totalBaseMetrics.NumDiffedCodeBytes += baseMetrics.NumCodeBytes; - totalDiffMetrics.NumDiffedCodeBytes += diffMetrics.NumCodeBytes; + totalBaseMetrics.Total.NumDiffedCodeBytes += baseMetrics.NumCodeBytes; + totalDiffMetrics.Total.NumDiffedCodeBytes += diffMetrics.NumCodeBytes; + + totalBaseMetricsOpts.NumDiffedCodeBytes += baseMetrics.NumCodeBytes; + totalDiffMetricsOpts.NumDiffedCodeBytes += diffMetrics.NumCodeBytes; + + totalBaseMetrics.Total.NumDiffExecutedInstructions += baseMetrics.NumExecutedInstructions; + totalDiffMetrics.Total.NumDiffExecutedInstructions += diffMetrics.NumExecutedInstructions; - totalBaseMetrics.NumDiffExecutedInstructions += baseMetrics.NumExecutedInstructions; - totalDiffMetrics.NumDiffExecutedInstructions += diffMetrics.NumExecutedInstructions; + totalBaseMetricsOpts.NumDiffExecutedInstructions += baseMetrics.NumExecutedInstructions; + totalDiffMetricsOpts.NumDiffExecutedInstructions += diffMetrics.NumExecutedInstructions; } } }