diff --git a/tests/test_flamechart.py b/tests/test_flamechart.py index eb6e92a5..d9c0b15e 100644 --- a/tests/test_flamechart.py +++ b/tests/test_flamechart.py @@ -88,16 +88,36 @@ class WindowTest(unittest.TestCase): class MedianRunTest(unittest.TestCase): + def median(self, runs): + """median_run over runs = {name: metrics}, with trace_report.measure answering from the dict.""" + measure = lambda paths, package, open_trace: [(p, runs[p]) for p in paths] + with mock.patch.object(flamechart.trace_report, "measure", measure): + return flamechart.median_run(list(runs), PKG, None) + def test_the_median_run_is_drawn_not_the_best(self): - scores = {"a": 90, "b": 300, "c": 80, "d": 95, "e": 85} - measure = lambda paths, package, open_trace: [(p, {"first frame": scores[p]}) for p in paths] - original = flamechart.trace_report.measure - flamechart.trace_report.measure = measure - try: - self.assertEqual(flamechart.median_run(list("abcde"), PKG, None), "a") - self.assertEqual(flamechart.median_run(list("abce"), PKG, None), "e") # 80 85 90 300: the lower middle - finally: - flamechart.trace_report.measure = original + runs = {k: {"first frame": v} for k, v in {"a": 90, "b": 300, "c": 80, "d": 95, "e": 85}.items()} + self.assertEqual(self.median(runs), ("a", "time to first frame")) + del runs["d"] + self.assertEqual(self.median(runs)[0], "e") # 80 85 90 300: the lower middle + + def test_nav_without_frame_timeline_uses_draw_time(self): + # #951's report: older Android has no FrameTimeline, so #953 leaves frame time out; the first file + # used to be drawn and called the median. + draws = {"nav-1": 300, "nav-2": 90, "nav-3": 80, "nav-4": 95, "nav-5": 85} + runs = {k: {"frames": 20, "draw median": v} for k, v in draws.items()} + self.assertEqual(self.median(runs), ("nav-2", "median RenderThread draw")) + + def test_a_measure_only_some_runs_have_is_not_mixed_with_another(self): + runs = {"a": {"frame median": 30, "draw median": 9}, "b": {"draw median": 7}, + "c": {"frame median": 10, "draw median": 8}} + self.assertEqual(self.median(runs), ("c", "median RenderThread draw")) + + def test_a_set_without_a_shared_measure_is_refused(self): + with self.assertRaises(ValueError): + self.median({"a": {"frames": 3}, "b": {"frames": 4}}) + + def test_one_trace_is_drawn_as_it_is(self): + self.assertEqual(flamechart.median_run(["only"], PKG, None), ("only", None)) class ArgumentTest(unittest.TestCase): diff --git a/tools/flamechart.md b/tools/flamechart.md index b8e64eb8..9a281ff1 100644 --- a/tools/flamechart.md +++ b/tools/flamechart.md @@ -15,7 +15,7 @@ tools/venv/bin/python tools/flamechart.py traces/before traces/after \ | Option | Default | Notes | |---|---|---| -| `TRACE ...` | | Before, then after (up to 4). A folder draws its median run (by time to first frame, or median frame time for `nav`), named in the panel. | +| `TRACE ...` | | Before, then after (up to 4). A folder draws its median run, by the first of time to first frame, median frame time and median RenderThread draw that every run has; the panel names the run and the measure. A folder with none of them in every run is refused. | | `--highlight REGEX` | `^(activityResume\|RV OnBindView)$` | Slice names to draw magenta and total. Children of a match count once. | | `--window` | `first-frame` | `first-frame`: launcher start to first frame drawn. `start:MS`: a fixed span from the start. `all`: the whole recording (always for `nav`). | | `--threads` | `main,RenderThread` | Lanes, by thread name in the launcher's process. | diff --git a/tools/flamechart.py b/tools/flamechart.py index 281a0fec..0c686379 100644 --- a/tools/flamechart.py +++ b/tools/flamechart.py @@ -8,8 +8,8 @@ visibly shorter. Slices whose name matches --highlight are drawn in magenta, and tools/flamechart.py traces/before traces/after --highlight '^(activityResume|RV OnBindView)$' \\ --title 'Back to Home: Home press to first frame' --out chart.png -BEFORE and AFTER are .pftrace files or folders; for a folder the median run is drawn (by the time to the -first frame, or for nav traces by the median frame time). Needs the optional packages in +BEFORE and AFTER are .pftrace files or folders; for a folder the median run is drawn, by the first of +time to first frame, median frame time and median RenderThread draw that every run has. Needs the optional packages in tools/requirements-perf.txt. Read tools/flamechart.md first. """ import argparse @@ -82,19 +82,28 @@ def highlighted(rows, rx, t0, t1): return total / MS, hit +# What a folder's median run is chosen by: the first of these that every run in the set has. +MEDIAN_BASES = (("first frame", "time to first frame"), ("frame median", "median frame time"), + ("draw median", "median RenderThread draw")) + + def median_run(paths, package, open_trace): - """The run whose time to the first frame (or nav frame time) is the median of the set.""" + """(path, basis): the middle run of the set by a measure every run has, and that measure's name. + + One trace is drawn as it is (basis None). A set with no measure common to every run is refused: + picking one of its runs and calling it the median would be wrong (#951). Nav traces from TVs without + FrameTimeline have no frame time, only RenderThread draw time (#953). + """ if len(paths) == 1: - return paths[0] - scored = [] - for path, metrics in zip(paths, (m for _, m in trace_report.measure(paths, package, open_trace))): - score = metrics.get("first frame", metrics.get("frame median")) - if score is not None: - scored.append((score, path)) - if not scored: - return paths[0] - scored.sort() - return scored[(len(scored) - 1) // 2][1] + return paths[0], None + measured = trace_report.measure(paths, package, open_trace) + for key, label in MEDIAN_BASES: + scores = [metrics.get(key) for _, metrics in measured] + if all(score is not None for score in scores): + ranked = sorted(zip(scores, paths)) + return ranked[(len(ranked) - 1) // 2][1], label + raise ValueError("no measure every run has (time to first frame, frame time or draw time); " + "pass a single trace to draw it as it is") def colour(name): @@ -224,7 +233,10 @@ def main(argv=None): paths = trace_report.expand([item]) if not paths: sys.exit(f"no .pftrace files in {item}") - path = median_run(paths, args.package, open_trace) + try: + path, basis = median_run(paths, args.package, open_trace) + except ValueError as e: + sys.exit(f"{item}: {e}") nav = trace_report.scenario_of(path) == "nav" q, close = open_trace(path) try: @@ -234,7 +246,7 @@ def main(argv=None): sys.exit(f"{path}: {e}") finally: close() - note = f"median of {len(paths)} runs ({os.path.basename(path)})" if len(paths) > 1 else os.path.basename(path) + note = f"median of {len(paths)} runs by {basis} ({os.path.basename(path)})" if basis else os.path.basename(path) panels.append({"label": labels[i] if i < len(labels) else f"Run {i + 1}", "note": note, "t0": t0, "t1": t1, "lanes": data}) size = draw(panels, args.rx, args.title, args.subtitle, args.out, args.width)