scripts: Adopted single folding pass, fixing perf[bd].py -r/--hot issue

There's an ordering issue with hotifying and folding when we have
multiple foldable results with children. This was hard to notice since
most of the recursive scripts have unique results, but it _is_ an issue
for perf.py/perfbd.py, which rely on result folding to merge samples.

The fix is to fold _before_ hotifying.

We could fold multiple times to avoid changing the behavior of the
result scripts, but instead I've just moved the folding in the table
renderer up into the relevant main functions. This means 1. we only fold
once, and 2. folding affects outputted csv/json files.

I'm a bit on the fence about this behavior change, but it is a bit more
consistent with how -r/--hot, -z/--depth, etc, affect both table and
csv/json results consistently.

Maybe we should move towards the table render always reflecting the
csv/json results? Most csv/json usage is with -q/--quiet anyways...

---

This does create a new risk in that the table renderer can hide results
if they aren't folded first.

To hopefully avoid this I've added an assert in the table renderer if it
notices results being hidden.
This commit is contained in:
Christopher Haster
2025-02-28 16:44:46 -06:00
parent 861dc3bd6a
commit 7789714560
9 changed files with 178 additions and 107 deletions

View File

@@ -609,15 +609,6 @@ def table(Result, results, diff_results=None, *,
fields = Result._fields
types = Result._types
# fold again, otherwise results risk being hidden
results = fold(Result, results,
by=by,
depth=depth)
if diff_results is not None:
diff_results = fold(Result, diff_results,
by=by,
depth=depth)
# organize by name
table = {
','.join(str(getattr(r, k)
@@ -632,6 +623,12 @@ def table(Result, results, diff_results=None, *,
for k in by): r
for r in diff_results or []}
# lost results? this only happens if we didn't fold by the same
# by field, which is an error and risks confusing results
assert len(table) == len(results)
if diff_results is not None:
assert len(diff_table) == len(diff_results)
# find compare entry if there is one
if compare:
compare_r = table.get(','.join(str(k) for k in compare))
@@ -1005,6 +1002,18 @@ def main(ci_paths,
depth=None,
hot=None,
**args):
# figure out what fields we're interested in
labels = None
if by is None:
if depth is not None or hot is not None:
by = ['z', 'function']
labels = ['function']
else:
by = ['function']
if fields is None:
fields = ['frame', 'limit']
# figure out depth
if depth is None:
depth = mt.inf if hot else 1
@@ -1076,10 +1085,10 @@ def main(ci_paths,
# print table
if not args.get('quiet'):
table(StackResult, results, diff_results,
by=by if by is not None else ['z', 'function'],
fields=fields if fields is not None else ['frame', 'limit'],
by=by,
fields=fields,
sort=sort,
labels=by if by is not None else ['function'],
labels=labels,
depth=depth,
**args)