From a3c01782d3e3110d1c49f1b682175f8aba3d9fa9 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 06:10:33 -0700 Subject: [PATCH 01/19] ci.eval.compare: format cmp-stats.py with ruff --- ci/eval/compare/cmp-stats.py | 60 +++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 0ef9c773163a..55e4d8ac98d9 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -8,6 +8,7 @@ from pathlib import Path # Define metrics of interest (can be expanded as needed) METRIC_PREFIXES = ("nr", "gc") + def flatten_data(json_data: dict) -> dict: """ Extracts and flattens metrics from JSON data. @@ -37,8 +38,6 @@ def flatten_data(json_data: dict) -> dict: return flat_metrics - - def load_all_metrics(directory: Path) -> dict: """ Loads all stats JSON files in the specified directory and extracts metrics. @@ -53,18 +52,19 @@ def load_all_metrics(directory: Path) -> dict: assert system_dir.is_dir() for chunk_output in system_dir.iterdir(): - with chunk_output.open() as f: - data = json.load(f) - metrics[f"{system_dir.name}/${chunk_output.name}"] = flatten_data(data) + with chunk_output.open() as f: + data = json.load(f) + metrics[f"{system_dir.name}/${chunk_output.name}"] = flatten_data(data) return metrics + def dataframe_to_markdown(df: pd.DataFrame) -> str: df = df.sort_values(by=df.columns[0], ascending=True) markdown_lines = [] # Header (get column names and format them) - header = '\n| ' + ' | '.join(df.columns) + ' |' + header = "\n| " + " | ".join(df.columns) + " |" markdown_lines.append(header) markdown_lines.append("| - " * (len(df.columns)) + "|") # Separator line @@ -78,21 +78,31 @@ def dataframe_to_markdown(df: pd.DataFrame) -> str: # Check for no change and NaN in p_value/t_stat row_values = [] for val in row: - if isinstance(val, float) and np.isnan(val): # For NaN values in p-value or t-stat + if isinstance(val, float) and np.isnan( + val + ): # For NaN values in p-value or t-stat row_values.append("-") # Custom symbol for NaN elif isinstance(val, float) and val == 0: # For no change (mean_diff == 0) row_values.append("-") # Custom symbol for no change else: - row_values.append(fmt(f"{val:.4f}" if isinstance(val, float) else str(val))) + row_values.append( + fmt(f"{val:.4f}" if isinstance(val, float) else str(val)) + ) - markdown_lines.append('| ' + ' | '.join(row_values) + ' |') + markdown_lines.append("| " + " | ".join(row_values) + " |") - return '\n'.join(markdown_lines) + return "\n".join(markdown_lines) def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.DataFrame: common_files = sorted(set(before_metrics) & set(after_metrics)) - all_keys = sorted({ metric_keys for file_metrics in before_metrics.values() for metric_keys in file_metrics.keys() }) + all_keys = sorted( + { + metric_keys + for file_metrics in before_metrics.values() + for metric_keys in file_metrics.keys() + } + ) results = [] @@ -112,15 +122,17 @@ def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.Data pct_change = 100 * diff / before_arr t_stat, p_val = ttest_rel(after_arr, before_arr) - results.append({ - "metric": key, - "mean_before": np.mean(before_arr), - "mean_after": np.mean(after_arr), - "mean_diff": np.mean(diff), - "mean_%_change": np.mean(pct_change), - "p_value": p_val, - "t_stat": t_stat - }) + results.append( + { + "metric": key, + "mean_before": np.mean(before_arr), + "mean_after": np.mean(after_arr), + "mean_diff": np.mean(diff), + "mean_%_change": np.mean(pct_change), + "p_value": p_val, + "t_stat": t_stat, + } + ) df = pd.DataFrame(results).sort_values("p_value") return df @@ -139,12 +151,16 @@ if __name__ == "__main__": # This may happen if the pull request target does not include PR#399720 yet. if not before_stats.exists(): - print("⚠️ Skipping comparison: stats directory is missing in the target commit.") + print( + "⚠️ Skipping comparison: stats directory is missing in the target commit." + ) exit(0) # This should never happen, but we're exiting gracefully anyways if not after_stats.exists(): - print("⚠️ Skipping comparison: stats directory missing in current PR evaluation.") + print( + "⚠️ Skipping comparison: stats directory missing in current PR evaluation." + ) exit(0) before_metrics = load_all_metrics(before_stats) From 6900cf62eebec068077a12493adbdcc06f43aa87 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 06:10:56 -0700 Subject: [PATCH 02/19] ci.eval.compare: sort imports in cmp-stats.py --- ci/eval/compare/cmp-stats.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 55e4d8ac98d9..73e852d82e5e 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -1,9 +1,10 @@ import json -import os -from scipy.stats import ttest_rel -import pandas as pd import numpy as np +import os +import pandas as pd + from pathlib import Path +from scipy.stats import ttest_rel # Define metrics of interest (can be expanded as needed) METRIC_PREFIXES = ("nr", "gc") From 4c2c6d9c43e4809096d5dca79cf4a8ad309cbcfe Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 06:11:47 -0700 Subject: [PATCH 03/19] ci.eval.compare: extract main function in cmp-stats.py --- ci/eval/compare/cmp-stats.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 73e852d82e5e..8a8546c197fb 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -139,7 +139,7 @@ def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.Data return df -if __name__ == "__main__": +def main(): before_dir = os.environ.get("BEFORE_DIR") after_dir = os.environ.get("AFTER_DIR") @@ -169,3 +169,7 @@ if __name__ == "__main__": df1 = perform_pairwise_tests(before_metrics, after_metrics) markdown_table = dataframe_to_markdown(df1) print(markdown_table) + + +if __name__ == "__main__": + main() From 2fe7b1cec2214baf504309575b2a533082914ff4 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 09:06:28 -0700 Subject: [PATCH 04/19] ci.eval.compare: extract a derivation for cmp-stats It's not very useful yet. --- ci/eval/compare/default.nix | 49 ++++++++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index 2c428a8ebd33..55aa7349f34b 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -5,7 +5,45 @@ runCommand, writeText, python3, + stdenvNoCC, + makeWrapper, }: +let + python = python3.withPackages (ps: [ + ps.numpy + ps.pandas + ps.scipy + ]); + + cmp-stats = stdenvNoCC.mkDerivation { + pname = "cmp-stats"; + version = lib.trivial.release; + + dontUnpack = true; + + nativeBuildInputs = [ makeWrapper ]; + + installPhase = '' + runHook preInstall + + mkdir -p $out/share/cmp-stats + + cp ${./cmp-stats.py} "$out/share/cmp-stats/cmp-stats.py" + + makeWrapper ${python.interpreter} "$out/bin/cmp-stats" \ + --add-flags "$out/share/cmp-stats/cmp-stats.py" + + runHook postInstall + ''; + + meta = { + description = "Performance comparison of Nix evaluation statistics"; + license = lib.licenses.mit; + mainProgram = "cmp-stats"; + maintainers = with lib.maintainers; [ philiptaron ]; + }; + }; +in { combinedDir, touchedFilesJson, @@ -140,14 +178,7 @@ runCommand "compare" # Don't depend on -dev outputs to reduce closure size for CI. nativeBuildInputs = map lib.getBin [ jq - (python3.withPackages ( - ps: with ps; [ - numpy - pandas - scipy - ] - )) - + cmp-stats ]; maintainers = builtins.toJSON maintainers; passAsFile = [ "maintainers" ]; @@ -181,7 +212,7 @@ runCommand "compare" echo } >> $out/step-summary.md - python3 ${./cmp-stats.py} >> $out/step-summary.md + cmp-stats >> $out/step-summary.md else # Package chunks are the same in both revisions From 241bb94b642afc4f4f2bbbb2ee5602c0f63df0c9 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 09:17:11 -0700 Subject: [PATCH 05/19] ci.eval.compare: use argument parsing instead of environment variables to pass before/after to cmp-stats.py --- ci/eval/compare/cmp-stats.py | 16 +++++++++------- ci/eval/compare/default.nix | 6 +----- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 8a8546c197fb..67f4ccf39961 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -1,3 +1,4 @@ +import argparse import json import numpy as np import os @@ -140,15 +141,16 @@ def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.Data def main(): - before_dir = os.environ.get("BEFORE_DIR") - after_dir = os.environ.get("AFTER_DIR") + parser = argparse.ArgumentParser( + description="Performance comparison of Nix evaluation statistics" + ) + parser.add_argument("before", help="directory containing baseline (data before)") + parser.add_argument("after", help="directory containing comparison (data after)") - if not before_dir or not after_dir: - print("Error: Environment variables 'BEFORE_DIR' and 'AFTER_DIR' must be set.") - exit(1) + options = parser.parse_args() - before_stats = Path(before_dir) / "stats" - after_stats = Path(after_dir) / "stats" + before_stats = Path(options.before) + after_stats = Path(options.after) # This may happen if the pull request target does not include PR#399720 yet. if not before_stats.exists(): diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index 55aa7349f34b..b0b56cce570a 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -182,10 +182,6 @@ runCommand "compare" ]; maintainers = builtins.toJSON maintainers; passAsFile = [ "maintainers" ]; - env = { - BEFORE_DIR = "${combined}/before"; - AFTER_DIR = "${combined}/after"; - }; } '' mkdir $out @@ -212,7 +208,7 @@ runCommand "compare" echo } >> $out/step-summary.md - cmp-stats >> $out/step-summary.md + cmp-stats ${combined}/before/stats ${combined}/after/stats >> $out/step-summary.md else # Package chunks are the same in both revisions From 4c7ec9bf2038df40b1885645fa83923ba798f13a Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 09:18:16 -0700 Subject: [PATCH 06/19] ci.eval.compare: require the directories to exist (they always should) --- ci/eval/compare/cmp-stats.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 67f4ccf39961..0b6bbb55af90 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -152,20 +152,6 @@ def main(): before_stats = Path(options.before) after_stats = Path(options.after) - # This may happen if the pull request target does not include PR#399720 yet. - if not before_stats.exists(): - print( - "⚠️ Skipping comparison: stats directory is missing in the target commit." - ) - exit(0) - - # This should never happen, but we're exiting gracefully anyways - if not after_stats.exists(): - print( - "⚠️ Skipping comparison: stats directory missing in current PR evaluation." - ) - exit(0) - before_metrics = load_all_metrics(before_stats) after_metrics = load_all_metrics(after_stats) df1 = perform_pairwise_tests(before_metrics, after_metrics) From f8210561f3d3a8d5a90fe6527cdbef6baa1091e1 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 11:59:13 -0700 Subject: [PATCH 07/19] ci.eval.compare: turn warnings into errors This helps detect my math errors --- ci/eval/compare/cmp-stats.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 0b6bbb55af90..8d256d10eb9c 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -3,6 +3,7 @@ import json import numpy as np import os import pandas as pd +import warnings from pathlib import Path from scipy.stats import ttest_rel @@ -149,6 +150,9 @@ def main(): options = parser.parse_args() + # Turn warnings into errors + warnings.simplefilter("error") + before_stats = Path(options.before) after_stats = Path(options.after) From 4bc54e7a3a9494b901453b4b7ecafc93b3b3dd23 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 12:03:37 -0700 Subject: [PATCH 08/19] ci.eval.compare: allow before_vals == 1 but avoid the t-test --- ci/eval/compare/cmp-stats.py | 39 +++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 8d256d10eb9c..87861dd84a03 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -117,25 +117,32 @@ def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.Data before_vals.append(before_metrics[fname][key]) after_vals.append(after_metrics[fname][key]) - if len(before_vals) >= 2: - before_arr = np.array(before_vals) - after_arr = np.array(after_vals) + if len(before_vals) == 0: + continue - diff = after_arr - before_arr - pct_change = 100 * diff / before_arr + before_arr = np.array(before_vals) + after_arr = np.array(after_vals) + + diff = after_arr - before_arr + pct_change = 100 * diff / before_arr + + # If there are enough values to perform a t-test, do so, otherwise mark NaN + if len(before_vals) == 1: + t_stat, p_val = [float("NaN")] * 2 + else: t_stat, p_val = ttest_rel(after_arr, before_arr) - results.append( - { - "metric": key, - "mean_before": np.mean(before_arr), - "mean_after": np.mean(after_arr), - "mean_diff": np.mean(diff), - "mean_%_change": np.mean(pct_change), - "p_value": p_val, - "t_stat": t_stat, - } - ) + results.append( + { + "metric": key, + "mean_before": np.mean(before_arr), + "mean_after": np.mean(after_arr), + "mean_diff": np.mean(diff), + "mean_%_change": np.mean(pct_change), + "p_value": p_val, + "t_stat": t_stat, + } + ) df = pd.DataFrame(results).sort_values("p_value") return df From 7818a245f794fb1f38e581cf93810cc0c6ee02a7 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 10:51:21 -0700 Subject: [PATCH 09/19] ci.eval.compare: support passing single files to cmp-stats --- ci/eval/compare/cmp-stats.py | 43 +++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 87861dd84a03..550d1e58a385 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -41,23 +41,36 @@ def flatten_data(json_data: dict) -> dict: return flat_metrics -def load_all_metrics(directory: Path) -> dict: +def load_all_metrics(path: Path) -> dict: """ - Loads all stats JSON files in the specified directory and extracts metrics. + Loads all stats JSON files in the specified file or directory and extracts metrics. + These stats JSON files are created by Nix when the `NIX_SHOW_STATS` environment variable is set. + + If the provided path is a directory, it must have the structure $path/$system/$stats, + where $path is the provided path, $system is some system from `lib.systems.doubles.*`, + and $stats is a stats JSON file. + + If the provided path is a file, it is a stats JSON file. Args: - directory (Path): Directory containing JSON files. + path (Path): Directory containing JSON files or a stats JSON file. + Returns: dict: Dictionary with filenames as keys and extracted metrics as values. """ metrics = {} - for system_dir in directory.iterdir(): - assert system_dir.is_dir() + if path.is_dir(): + for system_dir in path.iterdir(): + assert system_dir.is_dir() - for chunk_output in system_dir.iterdir(): - with chunk_output.open() as f: - data = json.load(f) - metrics[f"{system_dir.name}/${chunk_output.name}"] = flatten_data(data) + for chunk_output in system_dir.iterdir(): + with chunk_output.open() as f: + data = json.load(f) + + metrics[f"{system_dir.name}/${chunk_output.name}"] = flatten_data(data) + else: + with path.open() as f: + metrics[path.name] = flatten_data(json.load(f)) return metrics @@ -106,11 +119,11 @@ def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.Data for metric_keys in file_metrics.keys() } ) - results = [] for key in all_keys: - before_vals, after_vals = [], [] + before_vals = [] + after_vals = [] for fname in common_files: if key in before_metrics[fname] and key in after_metrics[fname]: @@ -152,8 +165,12 @@ def main(): parser = argparse.ArgumentParser( description="Performance comparison of Nix evaluation statistics" ) - parser.add_argument("before", help="directory containing baseline (data before)") - parser.add_argument("after", help="directory containing comparison (data after)") + parser.add_argument( + "before", help="File or directory containing baseline (data before)" + ) + parser.add_argument( + "after", help="File or directory containing comparison (data after)" + ) options = parser.parse_args() From e4101ea3a9a018c8ec7f50880ab82d5c0f795aa6 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 11:37:10 -0700 Subject: [PATCH 10/19] ci.eval.compare: instead of manually tabulating, use tabulate --- ci/eval/compare/cmp-stats.py | 34 +++++++++++++--------------------- ci/eval/compare/default.nix | 1 + 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 550d1e58a385..ea35fa87f1bc 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -7,6 +7,7 @@ import warnings from pathlib import Path from scipy.stats import ttest_rel +from tabulate import tabulate # Define metrics of interest (can be expanded as needed) METRIC_PREFIXES = ("nr", "gc") @@ -77,37 +78,28 @@ def load_all_metrics(path: Path) -> dict: def dataframe_to_markdown(df: pd.DataFrame) -> str: df = df.sort_values(by=df.columns[0], ascending=True) - markdown_lines = [] # Header (get column names and format them) - header = "\n| " + " | ".join(df.columns) + " |" - markdown_lines.append(header) - markdown_lines.append("| - " * (len(df.columns)) + "|") # Separator line + headers = [str(column) for column in df.columns] + table = [] # Iterate over rows to build Markdown rows for _, row in df.iterrows(): - # TODO: define threshold for highlighting - highlight = False - - fmt = lambda x: f"**{x}**" if highlight else f"{x}" - # Check for no change and NaN in p_value/t_stat row_values = [] for val in row: - if isinstance(val, float) and np.isnan( - val - ): # For NaN values in p-value or t-stat - row_values.append("-") # Custom symbol for NaN - elif isinstance(val, float) and val == 0: # For no change (mean_diff == 0) - row_values.append("-") # Custom symbol for no change + if isinstance(val, (float, int)): + if np.isnan(val): + row_values.append("-") # Custom symbol for NaN + elif val == 0: + row_values.append("-") # Custom symbol for no change + else: + row_values.append(f"{val:.4f}") else: - row_values.append( - fmt(f"{val:.4f}" if isinstance(val, float) else str(val)) - ) + row_values.append(str(val)) + table.append(row_values) - markdown_lines.append("| " + " | ".join(row_values) + " |") - - return "\n".join(markdown_lines) + return tabulate(table, headers, tablefmt="github") def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.DataFrame: diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index b0b56cce570a..7b9f03e602a8 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -13,6 +13,7 @@ let ps.numpy ps.pandas ps.scipy + ps.tabulate ]); cmp-stats = stdenvNoCC.mkDerivation { From 9959a4e507f8a69fda50e631715fd7ae00adf9f9 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 13:28:56 -0700 Subject: [PATCH 11/19] ci.eval.compare: delete unreferenced global --- ci/eval/compare/cmp-stats.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index ea35fa87f1bc..f4f8bdfadd44 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -9,9 +9,6 @@ from pathlib import Path from scipy.stats import ttest_rel from tabulate import tabulate -# Define metrics of interest (can be expanded as needed) -METRIC_PREFIXES = ("nr", "gc") - def flatten_data(json_data: dict) -> dict: """ From e83e900874e10e17a4ba1e4e7ae720f48268ec1f Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 13:29:38 -0700 Subject: [PATCH 12/19] ci.eval.compare: assert types in flatten_data --- ci/eval/compare/cmp-stats.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index f4f8bdfadd44..a7b4bea793da 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -30,12 +30,18 @@ def flatten_data(json_data: dict) -> dict: dict: Flattened metrics with keys as metric names. """ flat_metrics = {} - for k, v in json_data.items(): - if isinstance(v, (int, float)): - flat_metrics[k] = v - elif isinstance(v, dict): - for sub_k, sub_v in v.items(): - flat_metrics[f"{k}.{sub_k}"] = sub_v + for key, value in json_data.items(): + if isinstance(value, (int, float)): + flat_metrics[key] = value + elif isinstance(value, dict): + for subkey, subvalue in value.items(): + assert isinstance(subvalue, (int, float)), subvalue + flat_metrics[f"{key}.{subkey}"] = subvalue + else: + assert isinstance(value, (float, int, dict)), ( + f"Value `{value}` has unexpected type" + ) + return flat_metrics From 4eaa094f202006a7cefc10790c42139be73d847a Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 13:30:12 -0700 Subject: [PATCH 13/19] ci.eval.compare: sort time metrics first, then GC metrics, then everything else --- ci/eval/compare/cmp-stats.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index a7b4bea793da..069f54d7fb21 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -79,8 +79,19 @@ def load_all_metrics(path: Path) -> dict: return metrics +def metric_sort_key(name: str) -> str: + if name in ("cpuTime", "time.cpu", "time.gc", "time.gcFraction"): + return (1, name) + elif name.startswith("gc"): + return (2, name) + else: + return (3, name) + + def dataframe_to_markdown(df: pd.DataFrame) -> str: - df = df.sort_values(by=df.columns[0], ascending=True) + df = df.sort_values( + by=df.columns[0], ascending=True, key=lambda s: s.map(metric_sort_key) + ) # Header (get column names and format them) headers = [str(column) for column in df.columns] From 3edc1e204e76a564b9e0920be1ab336f703c7cb3 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 17:17:16 -0700 Subject: [PATCH 14/19] ci.eval.compare: make the table format using tabulate not manually --- ci/eval/compare/cmp-stats.py | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 069f54d7fb21..5bb54fea51c4 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -95,25 +95,16 @@ def dataframe_to_markdown(df: pd.DataFrame) -> str: # Header (get column names and format them) headers = [str(column) for column in df.columns] - table = [] + table = [ + [ + row["metric"], + # Check for no change and NaN in p_value/t_stat + *[None if np.isnan(val) or np.allclose(val, 0) else val for val in row[1:]], + ] + for _, row in df.iterrows() + ] - # Iterate over rows to build Markdown rows - for _, row in df.iterrows(): - # Check for no change and NaN in p_value/t_stat - row_values = [] - for val in row: - if isinstance(val, (float, int)): - if np.isnan(val): - row_values.append("-") # Custom symbol for NaN - elif val == 0: - row_values.append("-") # Custom symbol for no change - else: - row_values.append(f"{val:.4f}") - else: - row_values.append(str(val)) - table.append(row_values) - - return tabulate(table, headers, tablefmt="github") + return tabulate(table, headers, tablefmt="github", floatfmt=".4f", missingval="-") def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.DataFrame: From 210e3e115167d9876f2166a9938c0fc52d3646a7 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 17:46:16 -0700 Subject: [PATCH 15/19] ci.eval.compare: put things with bytes together --- ci/eval/compare/cmp-stats.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 5bb54fea51c4..7afb3a0bec1e 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -84,8 +84,10 @@ def metric_sort_key(name: str) -> str: return (1, name) elif name.startswith("gc"): return (2, name) - else: + elif name.endswith(("bytes", "Bytes")): return (3, name) + else: + return (4, name) def dataframe_to_markdown(df: pd.DataFrame) -> str: From 2817f796492587b12f63cf0a7ffb231b9937cb15 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Sep 2025 17:47:50 -0700 Subject: [PATCH 16/19] ci.eval.compare: put things with counts together --- ci/eval/compare/cmp-stats.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 7afb3a0bec1e..68ebd2d0236e 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -86,8 +86,10 @@ def metric_sort_key(name: str) -> str: return (2, name) elif name.endswith(("bytes", "Bytes")): return (3, name) - else: + elif name.startswith("nr") or name.endswith("number"): return (4, name) + else: + return (5, name) def dataframe_to_markdown(df: pd.DataFrame) -> str: From c9860ef95c7d87622f4d5a4c4158e3efae1a4a02 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Wed, 17 Sep 2025 11:30:39 -0700 Subject: [PATCH 17/19] ci.eval.compare: remove the duplicate cpuTime key --- ci/eval/compare/cmp-stats.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 68ebd2d0236e..7c536b6330c8 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -24,6 +24,9 @@ def flatten_data(json_data: dict) -> dict: "gc.heapSize": 5404549120 ... + See https://github.com/NixOS/nix/blob/187520ce88c47e2859064704f9320a2d6c97e56e/src/libexpr/eval.cc#L2846 + for the ultimate source of this data. + Args: json_data (dict): JSON data containing metrics. Returns: @@ -31,6 +34,10 @@ def flatten_data(json_data: dict) -> dict: """ flat_metrics = {} for key, value in json_data.items(): + # This key is duplicated as `time.cpu`; we keep that copy. + if key == "cpuTime": + continue + if isinstance(value, (int, float)): flat_metrics[key] = value elif isinstance(value, dict): @@ -80,7 +87,7 @@ def load_all_metrics(path: Path) -> dict: def metric_sort_key(name: str) -> str: - if name in ("cpuTime", "time.cpu", "time.gc", "time.gcFraction"): + if name in ("time.cpu", "time.gc", "time.gcFraction"): return (1, name) elif name.startswith("gc"): return (2, name) From fb1647ec6e02030b1ea0a2a84686f9cd49c02ace Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Wed, 17 Sep 2025 12:50:49 -0700 Subject: [PATCH 18/19] ci.eval.compare: explain the various metrics under the --explain flag --- ci/eval/compare/cmp-stats.py | 59 +++++++++++++++++++++++++++++++++--- ci/eval/compare/default.nix | 2 +- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index 7c536b6330c8..a456e721fffb 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -8,6 +8,7 @@ import warnings from pathlib import Path from scipy.stats import ttest_rel from tabulate import tabulate +from typing import Final def flatten_data(json_data: dict) -> dict: @@ -86,6 +87,49 @@ def load_all_metrics(path: Path) -> dict: return metrics +def metric_table_name(name: str, explain: bool) -> str: + """ + Returns the name of the metric, plus a footnote to explain it if needed. + """ + return f"{name}[^{name}]" if explain else name + + +METRIC_EXPLANATION_FOOTNOTE: Final[str] = """ + +[^time.cpu]: Number of seconds of CPU time accounted by the OS to the Nix evaluator process. On UNIX systems, this comes from [`getrusage(RUSAGE_SELF)`](https://man7.org/linux/man-pages/man2/getrusage.2.html). +[^time.gc]: Number of seconds of CPU time accounted by the Boehm garbage collector to performing GC. +[^time.gcFraction]: What fraction of the total CPU time is accounted towards performing GC. +[^gc.cycles]: Number of times garbage collection has been performed. +[^gc.heapSize]: Size in bytes of the garbage collector heap. +[^gc.totalBytes]: Size in bytes of all allocations in the garbage collector. +[^envs.bytes]: Size in bytes of all `Env` objects allocated by the Nix evaluator. These are almost exclusively created by [`nix-env`](https://nix.dev/manual/nix/stable/command-ref/nix-env.html). +[^list.bytes]: Size in bytes of all [lists](https://nix.dev/manual/nix/stable/language/syntax.html#list-literal) allocated by the Nix evaluator. +[^sets.bytes]: Size in bytes of all [attrsets](https://nix.dev/manual/nix/stable/language/syntax.html#list-literal) allocated by the Nix evaluator. +[^symbols.bytes]: Size in bytes of all items in the Nix evaluator symbol table. +[^values.bytes]: Size in bytes of all values allocated by the Nix evaluator. +[^envs.number]: The count of all `Env` objects allocated. +[^nrAvoided]: The number of thunks avoided being created. +[^nrExprs]: The number of expression objects ever created. +[^nrFunctionCalls]: The number of function calls ever made. +[^nrLookups]: The number of lookups into an attrset ever made. +[^nrOpUpdateValuesCopied]: The number of attrset values copied in the process of merging attrsets. +[^nrOpUpdates]: The number of attrsets merge operations (`//`) performed. +[^nrPrimOpCalls]: The number of function calls to primops (Nix builtins) ever made. +[^nrThunks]: The number of [thunks](https://nix.dev/manual/nix/latest/language/evaluation.html#laziness) ever made. A thunk is a delayed computation, represented by an expression reference and a closure. +[^sets.number]: The number of attrsets ever made. +[^symbols.number]: The number of symbols ever added to the symbol table. +[^values.number]: The number of values ever made. +[^envs.elements]: The number of values contained within an `Env` object. +[^list.concats]: The number of list concatenation operations (`++`) performed. +[^list.elements]: The number of values contained within a list. +[^sets.elements]: The number of values contained within an attrset. +[^sizes.Attr]: Size in bytes of the `Attr` type. +[^sizes.Bindings]: Size in bytes of the `Bindings` type. +[^sizes.Env]: Size in bytes of the `Env` type. +[^sizes.Value]: Size in bytes of the `Value` type. +""" + + def metric_sort_key(name: str) -> str: if name in ("time.cpu", "time.gc", "time.gcFraction"): return (1, name) @@ -99,7 +143,7 @@ def metric_sort_key(name: str) -> str: return (5, name) -def dataframe_to_markdown(df: pd.DataFrame) -> str: +def dataframe_to_markdown(df: pd.DataFrame, explain: bool) -> str: df = df.sort_values( by=df.columns[0], ascending=True, key=lambda s: s.map(metric_sort_key) ) @@ -108,14 +152,18 @@ def dataframe_to_markdown(df: pd.DataFrame) -> str: headers = [str(column) for column in df.columns] table = [ [ - row["metric"], + # The metric acts as its own footnote name + metric_table_name(row["metric"], explain), # Check for no change and NaN in p_value/t_stat *[None if np.isnan(val) or np.allclose(val, 0) else val for val in row[1:]], ] for _, row in df.iterrows() ] - return tabulate(table, headers, tablefmt="github", floatfmt=".4f", missingval="-") + result = tabulate(table, headers, tablefmt="github", floatfmt=".4f", missingval="-") + if explain: + result += METRIC_EXPLANATION_FOOTNOTE + return result def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.DataFrame: @@ -173,6 +221,9 @@ def main(): parser = argparse.ArgumentParser( description="Performance comparison of Nix evaluation statistics" ) + parser.add_argument( + "--explain", action="store_true", help="Explain the evaluation statistics" + ) parser.add_argument( "before", help="File or directory containing baseline (data before)" ) @@ -191,7 +242,7 @@ def main(): before_metrics = load_all_metrics(before_stats) after_metrics = load_all_metrics(after_stats) df1 = perform_pairwise_tests(before_metrics, after_metrics) - markdown_table = dataframe_to_markdown(df1) + markdown_table = dataframe_to_markdown(df1, explain=options.explain) print(markdown_table) diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index 7b9f03e602a8..3a025a0238f6 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -209,7 +209,7 @@ runCommand "compare" echo } >> $out/step-summary.md - cmp-stats ${combined}/before/stats ${combined}/after/stats >> $out/step-summary.md + cmp-stats --explain ${combined}/before/stats ${combined}/after/stats >> $out/step-summary.md else # Package chunks are the same in both revisions From d80d4a77b707cc6fe6839f274d443b736ce330db Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Wed, 17 Sep 2025 13:39:59 -0700 Subject: [PATCH 19/19] ci.eval.compare: split out equivalent values into their own table --- ci/eval/compare/cmp-stats.py | 163 +++++++++++++++++++++++++---------- 1 file changed, 117 insertions(+), 46 deletions(-) diff --git a/ci/eval/compare/cmp-stats.py b/ci/eval/compare/cmp-stats.py index a456e721fffb..cc34376ae381 100644 --- a/ci/eval/compare/cmp-stats.py +++ b/ci/eval/compare/cmp-stats.py @@ -5,6 +5,7 @@ import os import pandas as pd import warnings +from dataclasses import asdict, dataclass from pathlib import Path from scipy.stats import ttest_rel from tabulate import tabulate @@ -130,6 +131,88 @@ METRIC_EXPLANATION_FOOTNOTE: Final[str] = """ """ +@dataclass(frozen=True) +class PairwiseTestResults: + updated: pd.DataFrame + equivalent: pd.DataFrame + + @staticmethod + def tabulate(table, headers) -> str: + return tabulate( + table, headers, tablefmt="github", floatfmt=".4f", missingval="-" + ) + + def updated_to_markdown(self, explain: bool) -> str: + assert not self.updated.empty + # Header (get column names and format them) + return self.tabulate( + headers=[str(column) for column in self.updated.columns], + table=[ + [ + # The metric acts as its own footnote name + metric_table_name(row["metric"], explain), + # Check for no change and NaN in p_value/t_stat + *[ + None if np.isnan(val) or np.allclose(val, 0) else val + for val in row[1:] + ], + ] + for _, row in self.updated.iterrows() + ], + ) + + def equivalent_to_markdown(self, explain: bool) -> str: + assert not self.equivalent.empty + return self.tabulate( + headers=[str(column) for column in self.equivalent.columns], + table=[ + [ + # The metric acts as its own footnote name + metric_table_name(row["metric"], explain), + row["value"], + ] + for _, row in self.equivalent.iterrows() + ], + ) + + def to_markdown(self, explain: bool) -> str: + result = "" + + if not self.equivalent.empty: + result += "## Unchanged values\n\n" + result += self.equivalent_to_markdown(explain) + + if not self.updated.empty: + result += ("\n\n" if result else "") + "## Updated values\n\n" + result += self.updated_to_markdown(explain) + + if explain: + result += METRIC_EXPLANATION_FOOTNOTE + + return result + + +@dataclass(frozen=True) +class Equivalent: + metric: str + value: float + + +@dataclass(frozen=True) +class Comparison: + metric: str + mean_before: float + mean_after: float + mean_diff: float + mean_pct_change: float + + +@dataclass(frozen=True) +class ComparisonWithPValue(Comparison): + p_value: float + t_stat: float + + def metric_sort_key(name: str) -> str: if name in ("time.cpu", "time.gc", "time.gcFraction"): return (1, name) @@ -143,39 +226,21 @@ def metric_sort_key(name: str) -> str: return (5, name) -def dataframe_to_markdown(df: pd.DataFrame, explain: bool) -> str: - df = df.sort_values( - by=df.columns[0], ascending=True, key=lambda s: s.map(metric_sort_key) - ) - - # Header (get column names and format them) - headers = [str(column) for column in df.columns] - table = [ - [ - # The metric acts as its own footnote name - metric_table_name(row["metric"], explain), - # Check for no change and NaN in p_value/t_stat - *[None if np.isnan(val) or np.allclose(val, 0) else val for val in row[1:]], - ] - for _, row in df.iterrows() - ] - - result = tabulate(table, headers, tablefmt="github", floatfmt=".4f", missingval="-") - if explain: - result += METRIC_EXPLANATION_FOOTNOTE - return result - - -def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.DataFrame: +def perform_pairwise_tests( + before_metrics: dict, after_metrics: dict +) -> PairwiseTestResults: common_files = sorted(set(before_metrics) & set(after_metrics)) all_keys = sorted( { metric_keys for file_metrics in before_metrics.values() for metric_keys in file_metrics.keys() - } + }, + key=metric_sort_key, ) - results = [] + + updated = [] + equivalent = [] for key in all_keys: before_vals = [] @@ -193,28 +258,34 @@ def perform_pairwise_tests(before_metrics: dict, after_metrics: dict) -> pd.Data after_arr = np.array(after_vals) diff = after_arr - before_arr - pct_change = 100 * diff / before_arr - # If there are enough values to perform a t-test, do so, otherwise mark NaN - if len(before_vals) == 1: - t_stat, p_val = [float("NaN")] * 2 + # If there's no difference, add it all to the equivalent output. + if np.allclose(diff, 0): + equivalent.append(Equivalent(metric=key, value=before_vals[0])) else: - t_stat, p_val = ttest_rel(after_arr, before_arr) + pct_change = 100 * diff / before_arr - results.append( - { - "metric": key, - "mean_before": np.mean(before_arr), - "mean_after": np.mean(after_arr), - "mean_diff": np.mean(diff), - "mean_%_change": np.mean(pct_change), - "p_value": p_val, - "t_stat": t_stat, - } - ) + result = Comparison( + metric=key, + mean_before=np.mean(before_arr), + mean_after=np.mean(after_arr), + mean_diff=np.mean(diff), + mean_pct_change=np.mean(pct_change), + ) - df = pd.DataFrame(results).sort_values("p_value") - return df + # If there are enough values to perform a t-test, do so. + if len(before_vals) > 1: + t_stat, p_val = ttest_rel(after_arr, before_arr) + result = ComparisonWithPValue( + **asdict(result), p_value=p_val, t_stat=t_stat + ) + + updated.append(result) + + return PairwiseTestResults( + updated=pd.DataFrame(map(asdict, updated)), + equivalent=pd.DataFrame(map(asdict, equivalent)), + ) def main(): @@ -241,8 +312,8 @@ def main(): before_metrics = load_all_metrics(before_stats) after_metrics = load_all_metrics(after_stats) - df1 = perform_pairwise_tests(before_metrics, after_metrics) - markdown_table = dataframe_to_markdown(df1, explain=options.explain) + pairwise_test_results = perform_pairwise_tests(before_metrics, after_metrics) + markdown_table = pairwise_test_results.to_markdown(explain=options.explain) print(markdown_table)