From 7789714560cafa255c2c0596961c7a28a2979147 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 28 Feb 2025 16:44:46 -0600 Subject: [PATCH] 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. --- scripts/code.py | 24 +++++++++++++---------- scripts/cov.py | 36 ++++++++++++++++++++++------------- scripts/csv.py | 15 ++++++--------- scripts/ctx.py | 33 ++++++++++++++++++++------------ scripts/data.py | 24 +++++++++++++---------- scripts/perf.py | 47 ++++++++++++++++++++++++++++++---------------- scripts/perfbd.py | 40 ++++++++++++++++++++++++++------------- scripts/stack.py | 33 ++++++++++++++++++++------------ scripts/structs.py | 33 ++++++++++++++++++++------------ 9 files changed, 178 insertions(+), 107 deletions(-) diff --git a/scripts/code.py b/scripts/code.py index d668f0cc..17a91599 100755 --- a/scripts/code.py +++ b/scripts/code.py @@ -605,15 +605,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) @@ -628,6 +619,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)) @@ -999,6 +996,13 @@ def main(obj_paths, *, defines=[], sort=None, **args): + # figure out what fields we're interested in + if by is None: + by = ['function'] + + if fields is None: + fields = ['size'] + # find sizes if not args.get('use', None): # not enough info? @@ -1051,7 +1055,7 @@ def main(obj_paths, *, # print table if not args.get('quiet'): table(CodeResult, results, diff_results, - by=by if by is not None else ['function'], + by=by, fields=fields, sort=sort, **args) diff --git a/scripts/cov.py b/scripts/cov.py index e35a11e4..37ac7b72 100755 --- a/scripts/cov.py +++ b/scripts/cov.py @@ -466,15 +466,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) @@ -489,6 +480,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)) @@ -944,6 +941,21 @@ def main(gcda_paths, *, else: args['color'] = False + # figure out what fields we're interested in + if by is None: + if (args.get('annotate') + or args.get('lines') + or args.get('branches')): + by = ['file', 'line'] + else: + by = ['function'] + + if fields is None: + if not hits: + fields = ['lines', 'branches'] + else: + fields = ['calls', 'hits'] + # find sizes if not args.get('use', None): # not enough info? @@ -1003,10 +1015,8 @@ def main(gcda_paths, *, else: # print table table(CovResult, results, diff_results, - by=by if by is not None else ['function'], - fields=fields if fields is not None - else ['lines', 'branches'] if not hits - else ['calls', 'hits'], + by=by, + fields=fields, sort=sort, **args) diff --git a/scripts/csv.py b/scripts/csv.py index ff98b6dd..5993a62a 100755 --- a/scripts/csv.py +++ b/scripts/csv.py @@ -1736,15 +1736,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) @@ -1759,6 +1750,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)) diff --git a/scripts/ctx.py b/scripts/ctx.py index 76e275b5..d5ba5067 100755 --- a/scripts/ctx.py +++ b/scripts/ctx.py @@ -863,15 +863,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) @@ -886,6 +877,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)) @@ -1259,6 +1256,18 @@ def main(obj_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', 'i', 'function'] + labels = ['function'] + else: + by = ['function'] + + if fields is None: + fields = ['size'] + # figure out depth if depth is None: depth = mt.inf if hot else 1 @@ -1330,10 +1339,10 @@ def main(obj_paths, *, # print table if not args.get('quiet'): table(CtxResult, results, diff_results, - by=by if by is not None else ['z', 'i', 'function'], - fields=fields if fields is not None else ['size'], + by=by, + fields=fields, sort=sort, - labels=by if by is not None else ['function'], + labels=labels, depth=depth, **args) diff --git a/scripts/data.py b/scripts/data.py index f77301f7..00ad4ee0 100755 --- a/scripts/data.py +++ b/scripts/data.py @@ -605,15 +605,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) @@ -628,6 +619,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)) @@ -999,6 +996,13 @@ def main(obj_paths, *, defines=[], sort=None, **args): + # figure out what fields we're interested in + if by is None: + by = ['function'] + + if fields is None: + fields = ['size'] + # find sizes if not args.get('use', None): # not enough info? @@ -1051,7 +1055,7 @@ def main(obj_paths, *, # print table if not args.get('quiet'): table(DataResult, results, diff_results, - by=by if by is not None else ['function'], + by=by, fields=fields, sort=sort, **args) diff --git a/scripts/perf.py b/scripts/perf.py index 7fc27312..20474eca 100755 --- a/scripts/perf.py +++ b/scripts/perf.py @@ -967,15 +967,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) @@ -990,6 +981,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)) @@ -1472,6 +1469,26 @@ def report(perf_paths, *, else: args['color'] = False + # figure out what fields we're interested in + labels = None + if by is None: + if (args.get('annotate') + or args.get('threshold')): + by = ['file', 'line'] + elif depth is not None or hot is not None: + by = ['z', 'function'] + labels = ['function'] + else: + by = ['function'] + + if fields is None: + if not branches and not caches: + fields = ['cycles'] + elif branches: + fields = ['bmisses', 'branches'] + else: + fields = ['cmisses', 'caches'] + # figure out depth if depth is None: depth = mt.inf if hot else 1 @@ -1542,7 +1559,8 @@ def report(perf_paths, *, # print table if not args.get('quiet'): - if args.get('annotate') or args.get('threshold'): + if (args.get('annotate') + or args.get('threshold')): # annotate sources annotate(PerfResult, results, branches=branches, @@ -1551,13 +1569,10 @@ def report(perf_paths, *, else: # print table table(PerfResult, results, diff_results, - by=by if by is not None else ['z', 'function'], - fields=fields if fields is not None - else ['cycles'] if not branches and not caches - else ['bmisses', 'branches'] if branches - else ['cmisses', 'caches'], + by=by, + fields=fields, sort=sort, - labels=by if by is not None else ['function'], + labels=labels, depth=depth, **args) diff --git a/scripts/perfbd.py b/scripts/perfbd.py index d095d3f0..9c501504 100755 --- a/scripts/perfbd.py +++ b/scripts/perfbd.py @@ -937,15 +937,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) @@ -960,6 +951,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)) @@ -1454,6 +1451,24 @@ def report(paths, *, else: args['color'] = False + # figure out what fields we're interested in + labels = None + if by is None: + if (args.get('annotate') + or args.get('threshold') + or args.get('read_threshold') + or args.get('prog_threshold') + or args.get('erase_threshold')): + by = ['file', 'line'] + elif depth is not None or hot is not None: + by = ['z', 'function'] + labels = ['function'] + else: + by = ['function'] + + if fields is None: + fields = ['readed', 'proged', 'erased'] + # figure out depth if depth is None: depth = mt.inf if hot else 1 @@ -1551,11 +1566,10 @@ def report(paths, *, else: # print table table(PerfBdResult, results, diff_results, - by=by if by is not None else ['z', 'function'], - fields=fields if fields is not None - else ['readed', 'proged', 'erased'], + by=by, + fields=fields, sort=sort, - labels=by if by is not None else ['function'], + labels=labels, depth=depth, **args) diff --git a/scripts/stack.py b/scripts/stack.py index a9f66c9b..6ce6ecdb 100755 --- a/scripts/stack.py +++ b/scripts/stack.py @@ -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) diff --git a/scripts/structs.py b/scripts/structs.py index c8c71423..6a3ebb94 100755 --- a/scripts/structs.py +++ b/scripts/structs.py @@ -683,15 +683,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) @@ -706,6 +697,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)) @@ -1079,6 +1076,18 @@ def main(obj_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', 'i', 'struct'] + labels = ['struct'] + else: + by = ['struct'] + + if fields is None: + fields = ['size', 'align'] + # figure out depth if depth is None: depth = mt.inf if hot else 1 @@ -1150,10 +1159,10 @@ def main(obj_paths, *, # print table if not args.get('quiet'): table(StructResult, results, diff_results, - by=by if by is not None else ['z', 'i', 'struct'], - fields=fields if fields is not None else ['size', 'align'], + by=by, + fields=fields, sort=sort, - labels=by if by is not None else ['struct'], + labels=labels, depth=depth, **args)