From 3bec33b74c3cee23039fb7ca0d00a7e30efb6260 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 6 Feb 2023 19:37:53 +0100 Subject: [PATCH] SPMI: Display TP diffs as a range in the header (#81661) The current approach of averaging all collections does not make sense. --- src/coreclr/scripts/superpmi.py | 144 +++++++++++++++++--------------- 1 file changed, 77 insertions(+), 67 deletions(-) diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index 4c50031bc9007..5c4811f1bd811 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -2324,85 +2324,95 @@ def replay_with_throughput_diff(self): os.remove(overall_md_summary_file) with open(overall_md_summary_file, "w") as write_fh: - if base_jit_compiler_version != diff_jit_compiler_version: - write_fh.write("Warning: Different compilers used for base and diff JITs. Results may be misleading.\n") - write_fh.write("Base JIT's compiler: {}\n".format(base_jit_compiler_version)) - write_fh.write("Diff JIT's compiler: {}\n".format(diff_jit_compiler_version)) - - # 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_pct(base, diff): - if base == 0: - return diff != 0 - - return round((diff - base) / base * 100, 2) != 0 - - def is_significant(row, base, diff): - return is_significant_pct(int(base[row]["Diff executed instructions"]), int(diff[row]["Diff executed instructions"])) - def format_pct(base_instructions, diff_instructions): - plus_if_positive = "+" if diff_instructions > base_instructions else "" - if base_instructions > 0: - pct = (diff_instructions - base_instructions) / base_instructions * 100 - else: - pct = 0.0 + self.write_tpdiff_markdown_summary(write_fh, tp_diffs, base_jit_compiler_version, diff_jit_compiler_version) + logging.info(" Summary Markdown file: %s", overall_md_summary_file) - text = "{}{:.2f}%".format(plus_if_positive, pct) - if diff_instructions != base_instructions: - color = "red" if diff_instructions > base_instructions else "green" - return html_color(color, text) + return True + ################################################################################################ end of replay_with_throughput_diff() - return text + def write_tpdiff_markdown_summary(self, write_fh, tp_diffs, base_jit_compiler_version, diff_jit_compiler_version): + if base_jit_compiler_version != diff_jit_compiler_version: + write_fh.write("Warning: Different compilers used for base and diff JITs. Results may be misleading.\n") + write_fh.write("Base JIT's compiler: {}\n".format(base_jit_compiler_version)) + write_fh.write("Diff JIT's compiler: {}\n".format(diff_jit_compiler_version)) - if any(is_significant(row, base, diff) for row in ["Overall", "MinOpts", "FullOpts"] for (_, base, diff) in tp_diffs): - def write_pivot_section(row): - if not any(is_significant(row, base, diff) for (_, base, diff) in tp_diffs): - return + # 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_pct(base, diff): + if base == 0: + return diff != 0 - write_fh.write("\n
\n") - sum_base = sum(int(base_metrics[row]["Diff executed instructions"]) for (_, base_metrics, _) in tp_diffs) - sum_diff = sum(int(diff_metrics[row]["Diff executed instructions"]) for (_, _, diff_metrics) in tp_diffs) + return round((diff - base) / base * 100, 2) != 0 - write_fh.write("{} ({})\n\n".format(row, format_pct(sum_base, sum_diff))) - write_fh.write("|Collection|PDIFF|\n") - write_fh.write("|---|--:|\n") - for mch_file, base, diff in tp_diffs: - base_instructions = int(base[row]["Diff executed instructions"]) - diff_instructions = int(diff[row]["Diff executed instructions"]) + def is_significant(row, base, diff): + return is_significant_pct(int(base[row]["Diff executed instructions"]), int(diff[row]["Diff executed instructions"])) - if is_significant(row, base, diff): - write_fh.write("|{}|{}|\n".format( - mch_file, - format_pct(base_instructions, diff_instructions))) + def compute_pct(base_instructions, diff_instructions): + if base_instructions > 0: + return (diff_instructions - base_instructions) / base_instructions * 100 + else: + return 0.0 - write_fh.write("\n\n
\n") + def format_pct(pct): + plus_if_positive = "+" if pct > 0 else "" - write_pivot_section("Overall") - write_pivot_section("MinOpts") - write_pivot_section("FullOpts") + text = "{}{:.2f}%".format(plus_if_positive, pct) + if pct != 0: + color = "red" if pct > 0 else "green" + return html_color(color, text) - else: - write_fh.write("No significant throughput differences found\n") + return text - write_fh.write("\n
\n") - write_fh.write("Details\n\n") - for (disp, row) in [("All", "Overall"), ("MinOpts", "MinOpts"), ("FullOpts", "FullOpts")]: - 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[row]["Diff executed instructions"]) - diff_instructions = int(diff[row]["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") + def compute_and_format_pct(base_instructions, diff_instructions): + return format_pct(compute_pct(base_instructions, diff_instructions)) - logging.info(" Summary Markdown file: %s", overall_md_summary_file) + if any(is_significant(row, base, diff) for row in ["Overall", "MinOpts", "FullOpts"] for (_, base, diff) in tp_diffs): + def write_pivot_section(row): + if not any(is_significant(row, base, diff) for (_, base, diff) in tp_diffs): + return - return True - ################################################################################################ end of replay_with_throughput_diff() + write_fh.write("\n
\n") + pcts = [compute_pct(int(base_metrics[row]["Diff executed instructions"]), int(diff_metrics[row]["Diff executed instructions"])) for (_, base_metrics, diff_metrics) in tp_diffs] + min_pct_str = format_pct(min(pcts)) + max_pct_str = format_pct(max(pcts)) + if min_pct_str == max_pct_str: + write_fh.write("{} ({})\n\n".format(row, min_pct_str)) + else: + write_fh.write("{} ({} to {})\n\n".format(row, min_pct_str, max_pct_str)) + write_fh.write("|Collection|PDIFF|\n") + write_fh.write("|---|--:|\n") + for mch_file, base, diff in tp_diffs: + base_instructions = int(base[row]["Diff executed instructions"]) + diff_instructions = int(diff[row]["Diff executed instructions"]) + + if is_significant(row, base, diff): + write_fh.write("|{}|{}|\n".format( + mch_file, + compute_and_format_pct(base_instructions, diff_instructions))) + + write_fh.write("\n\n
\n") + + write_pivot_section("Overall") + write_pivot_section("MinOpts") + write_pivot_section("FullOpts") + else: + write_fh.write("No significant throughput differences found\n") + + write_fh.write("\n
\n") + write_fh.write("Details\n\n") + for (disp, row) in [("All", "Overall"), ("MinOpts", "MinOpts"), ("FullOpts", "FullOpts")]: + 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[row]["Diff executed instructions"]) + diff_instructions = int(diff[row]["Diff executed instructions"]) + write_fh.write("|{}|{:,d}|{:,d}|{}|\n".format( + mch_file, base_instructions, diff_instructions, + compute_and_format_pct(base_instructions, diff_instructions))) + write_fh.write("\n") + write_fh.write("\n
\n") ################################################################################ # Argument handling helpers