Merge pull request #149 from faster-cpython/better-tables

mdboom · web-flow · commit b92fb4d7cb04 · 2024-03-07T10:21:39.000-05:00
Improve legibility of navigation tables
diff --git a/bench_runner/result.py b/bench_runner/result.py
@@ -119,19 +119,11 @@ def geometric_mean(self) -> str:
 
         lines = self.contents_lines
 
-        if (
-            self.head.benchmark_hash is None
-            or self.ref.benchmark_hash != self.head.benchmark_hash
-        ):
-            suffix = r" \*"
-        else:
-            suffix = ""
-
         # We want to get the *last* geometric mean in the file, in case
         # it's divided by tags
         for line in lines[::-1]:
             if "Geometric mean" in line:
-                geometric_mean = line.split("|")[3].strip() + suffix
+                geometric_mean = line.split("|")[3].strip()
                 break
         else:
             geometric_mean = "not sig"
@@ -224,13 +216,8 @@ def summary(self) -> str:
             return ""
 
         result = self.geometric_mean
-        reliability = self.hpt_reliability
-        if reliability is not None:
-            reliability = reliability[:-4]
-            result += f" ({reliability}%)"
-        memory_change = self.memory_change
-        if memory_change not in (None, "unknown"):
-            result += f" ({memory_change} m)"
+        result = result.replace("faster", "↑")
+        result = result.replace("slower", "↓")
 
         return result
 
@@ -626,9 +613,9 @@ def hash_and_flags(self) -> str:
         # A representation for the user that combines the commit hash and other flags
         parts = [self.cpython_hash]
         if self.is_tier2:
-            parts.append("2️⃣")
+            parts.append("️(T2)")
         if self.is_jit:
-            parts.append("*️⃣")
+            parts.append("(JIT)")
         return " ".join(parts)
 
     @functools.cached_property
diff --git a/bench_runner/scripts/compare.py b/bench_runner/scripts/compare.py
@@ -65,7 +65,11 @@ def compare_pair(
         ("slower", "faster"),
     )
 
-    return f"{comparison.summary} [table]({name}.md) [plot]({name}.png)"
+    return (
+        comparison.summary
+        + f" [{util.TYPE_TO_ICON['table']}]({name}.md)"
+        + f" [{util.TYPE_TO_ICON['time plot']}]({name}.png)"
+    )
 
 
 def write_row(fd, columns: list[str]):
diff --git a/bench_runner/scripts/generate_results.py b/bench_runner/scripts/generate_results.py
@@ -164,22 +164,35 @@ def output_results_index(
     """
     bases = [*bases, "base"]
 
-    head = ["date", "fork", "ref", "version", "hash"] + [
-        f"vs. {base}:" for base in bases
-    ]
+    head = ["date", "fork/ref", "hash/flags"] + [f"vs. {base}:" for base in bases]
 
     rows = []
     for result in results:
         versus = []
         for base in bases:
             if base in result.bases and result.bases[base].valid_comparison:
-                versus.append(
-                    table.md_link(
-                        result.bases[base].summary,
-                        result.bases[base].filename.with_suffix(".md"),
+                compare = result.bases[base]
+                entry = (
+                    compare.summary
+                    + "<br>"
+                    + table.md_link(
+                        util.TYPE_TO_ICON["table"],
+                        compare.filename.with_suffix(".md"),
+                        filename,
+                    )
+                    + table.md_link(
+                        util.TYPE_TO_ICON["time plot"],
+                        compare.filename.with_suffix(".png"),
                         filename,
                     )
                 )
+                if base == "base" and compare.memory_change not in (None, "unknown"):
+                    entry += table.md_link(
+                        util.TYPE_TO_ICON["memory plot"],
+                        compare.filename.parent / (compare.filename.stem + "-mem.png"),
+                        filename,
+                    )
+                versus.append(entry)
             else:
                 versus.append("")
 
@@ -188,9 +201,7 @@ def output_results_index(
                 table.md_link(
                     result.commit_date, str(result.filename.parent), filename
                 ),
-                unquote(result.fork),
-                result.ref[:10],
-                result.version,
+                f"{unquote(result.fork)}/{result.ref}",
                 result.hash_and_flags,
                 *versus,
             ]
@@ -393,7 +404,9 @@ def get_directory_indices_entries(
                         dirpath,
                         result.runner,
                         base,
-                        table.md_link(type, result.filename.name),
+                        table.md_link(
+                            util.TYPE_TO_ICON.get(type, "") + type, result.filename.name
+                        ),
                     )
                 )
 
diff --git a/bench_runner/util.py b/bench_runner/util.py
@@ -15,3 +15,10 @@ def get_benchmark_hash() -> str:
     hash.update(os.environ["PYPERFORMANCE_HASH"].encode("ascii")[:7])
     hash.update(os.environ["PYSTON_BENCHMARKS_HASH"].encode("ascii")[:7])
     return hash.hexdigest()[:6]
+
+
+TYPE_TO_ICON = {
+    "table": "📄",
+    "time plot": "📈",
+    "memory plot": "🧠",
+}
diff --git a/tests/test_compare.py b/tests/test_compare.py
@@ -38,8 +38,8 @@ def test_compare_1_to_n(tmp_path, monkeypatch):
 
         | commit | change |
         | -- | -- |
-        | eb0004c (eb0004c) | 1.57x faster (94%) (1.11x m) [table](linux-eb0004c-vs-9d38120.md) [plot](linux-eb0004c-vs-9d38120.png) |
-        | b0e1f9c (b0e1f9c) | 1.70x faster \\* (94%) (1.21x m) [table](linux-b0e1f9c-vs-9d38120.md) [plot](linux-b0e1f9c-vs-9d38120.png) |
+        | eb0004c (eb0004c) | 1.57x ↑ [📄](linux-eb0004c-vs-9d38120.md) [📈](linux-eb0004c-vs-9d38120.png) |
+        | b0e1f9c (b0e1f9c) | 1.70x ↑ [📄](linux-b0e1f9c-vs-9d38120.md) [📈](linux-b0e1f9c-vs-9d38120.png) |
         """  # noqa
     ).strip()
     assert expected in content
@@ -72,9 +72,9 @@ def test_compare_n_to_n(tmp_path, monkeypatch):
 
         |  | 9d38120 (9d38120) | eb0004c (eb0004c) | b0e1f9c (b0e1f9c) |
         | -- | -- | -- | -- |
-        | 9d38120 |  | 1.57x faster (94%) (1.11x m) [table](linux-eb0004c-vs-9d38120.md) [plot](linux-eb0004c-vs-9d38120.png) | 1.70x faster \* (94%) (1.21x m) [table](linux-b0e1f9c-vs-9d38120.md) [plot](linux-b0e1f9c-vs-9d38120.png) |
-        | eb0004c | 1.57x slower (94%) (0.90x m) [table](linux-9d38120-vs-eb0004c.md) [plot](linux-9d38120-vs-eb0004c.png) |  | 1.08x faster \* (94%) (1.09x m) [table](linux-b0e1f9c-vs-eb0004c.md) [plot](linux-b0e1f9c-vs-eb0004c.png) |
-        | b0e1f9c | 1.70x slower \\* (94%) (0.83x m) [table](linux-9d38120-vs-b0e1f9c.md) [plot](linux-9d38120-vs-b0e1f9c.png) | 1.08x slower \* (94%) (0.92x m) [table](linux-eb0004c-vs-b0e1f9c.md) [plot](linux-eb0004c-vs-b0e1f9c.png) |  |
+        | 9d38120 |  | 1.57x ↑ [📄](linux-eb0004c-vs-9d38120.md) [📈](linux-eb0004c-vs-9d38120.png) | 1.70x ↑ [📄](linux-b0e1f9c-vs-9d38120.md) [📈](linux-b0e1f9c-vs-9d38120.png) |
+        | eb0004c | 1.57x ↓ [📄](linux-9d38120-vs-eb0004c.md) [📈](linux-9d38120-vs-eb0004c.png) |  | 1.08x ↑ [📄](linux-b0e1f9c-vs-eb0004c.md) [📈](linux-b0e1f9c-vs-eb0004c.png) |
+        | b0e1f9c | 1.70x ↓ [📄](linux-9d38120-vs-b0e1f9c.md) [📈](linux-9d38120-vs-b0e1f9c.png) | 1.08x ↓ [📄](linux-eb0004c-vs-b0e1f9c.md) [📈](linux-eb0004c-vs-b0e1f9c.png) |  |
         """  # noqa
     ).strip()
     assert expected in content
diff --git a/tests/test_generate_results.py b/tests/test_generate_results.py
@@ -105,39 +105,6 @@ def test_main(tmp_path, monkeypatch):
     _run_for_bases(["3.10.4", "3.11.0b3"], repo_path, has_base=["b0e1f9c"])
     _run_for_bases(["3.10.4", "3.11.0b3"], repo_path, has_base=["b0e1f9c"])
 
-    rows = _parse_table(repo_path / "README.md")
-    versions = [row[3] for row in rows]
-    assert len(set(versions)) == len(versions)
-    print(versions)
-    assert versions == [
-        "version",
-        "---",
-        "3.12.0a3+",
-        "3.12.0a2+",
-        "3.12.0a1+",
-        "3.11.0b3",
-        "3.10.4",
-    ]
-
-    rows = _parse_table(repo_path / "RESULTS.md")
-    versions = [row[3] for row in rows]
-    assert len(set(versions)) != len(versions)
-    assert versions == [
-        "version",
-        "---",
-        "3.12.0a3+",
-        "3.12.0a2+",
-        "3.12.0a1+",
-        "3.12.0a1+",
-        "3.11.0b3",
-        "3.11.0b2",
-        "3.11.0b1",
-        "3.11.0a7",
-        "3.11.0a6",
-        "3.11.0a3",
-        "3.10.4",
-    ]
-
 
 def test_change_bases(tmp_path):
     repo_path = _copy_repo(tmp_path)
diff --git a/tests/test_result.py b/tests/test_result.py
@@ -70,7 +70,7 @@ def test_merge_base(tmp_path):
     assert head.commit_merge_base == "9d38120e335357a3b294277fd5eff0a10e46e043"
     assert comparison.ref.version == "3.10.4"
     assert comparison.head is head
-    assert comparison.geometric_mean == "1.70x faster \\*"
+    assert comparison.geometric_mean == "1.70x faster"
 
 
 def test_from_scratch(monkeypatch):
