Re: [PATCH v3 1/2] perf python: New treport script

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The script relies on missing `perf` Python C extension APIs, causing immediate crashes on import or execution.
- [Low] The argparse description inaccurately claims the script creates flame graphs.
- [Low] Typo in the source code comments ('profle' instead of 'profile').
--

--- Patch [1]: [PATCH v3 1/2] perf python: New treport script ---
commit d9d175087e8fce3c5649b6249af90e23002fff56
Author: Alice Rogers <[email protected]>

perf python: New treport script

This commit adds a new treport.py script that uses the textual framework to
display an interactive tree report of processed perf samples.

> diff --git a/tools/perf/python/treport.py b/tools/perf/python/treport.py
> new file mode 100755
> index 0000000000000..528a640e4d356
> --- /dev/null
> +++ b/tools/perf/python/treport.py
[ ... ]
> +from textual.widgets import Footer, Header, TabbedContent, TabPane, Tree
> +from textual.widgets.tree import TreeNode
> +
> +# Global session.
> +session :Optional[perf.session] = None
> +

[Severity: High]
Are there prerequisite patches missing from this series? The perf Python C
extension does not currently expose perf.session. Because this type annotation
is evaluated at module load time, running the script results in an immediate
AttributeError.

[ ... ]
> +        node.value += period
> +
> +        if sample.callchain:
> +            for entry in reversed(sample.callchain):
> +                name = entry.symbol
> +                if not name or name == "[unknown]":

[Severity: High]
Does the Python sample_event object expose callchain and symbol? It appears
these attributes are also missing from the current Python C extension, which
would cause this to crash when processing samples.

[ ... ]
> +class ProfileBuilder:
> +    """Constructs a profile tree from a stream of events."""
> +    def __init__(self):
> +        self.root = ProfileNode("root", parent=None)
> +
> +    def process_event(self, sample) -> None:
> +        """Called by session.process_events to update the profile tree."""
> +        ev_name = str(sample.evsel)[6:-1]

[Severity: High]
Does the sample object expose evsel? Similar to the other missing attributes,
this seems to rely on Python C extension features that are not yet present in
the tree.

[ ... ]
> +if __name__ == "__main__":
> +    parser = argparse.ArgumentParser(description="Create flame graphs using perf python module.")
> +    parser.add_argument("-i", "--input", help="input perf.data file")

[Severity: Low]
This isn't a bug, but the description says "Create flame graphs" instead
of describing the interactive tree report this script actually generates.
Is this a copy-paste error from another script?

[ ... ]
> +    except Exception as e:
> +        print(f"Error opening session: {e}", file=sys.stderr)
> +        sys.exit(1)
> +
> +    # profle.process_event is called for each perf event to build the profile.
> +    session.process_events()

[Severity: Low]
This isn't a bug, but there is a typo (profle) in this comment.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1