Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polish perun import and viewdiff #257

Merged

Conversation

JiriPavela
Copy link
Collaborator

This PR addresses several issues with the current perun import and viewdiff modules. Among other things:

  • Stats and metadata specification, internal representation and visualization have been unified across different perf and ELK import and viewdiff commands.
  • Perf profile stats may now be specified both on CLI and in CSV files.
  • Stats may now be aggregated in different ways depending on the type of stats value(s), aggregation key and comparison operator.
  • Aggregated stats are now displayed in a nested table in viewdiff.
  • Metadata may now be specified directly on CLI using strings or JSON files.
  • Collapsing or showing different parts of profile headers in viewdiff now collapses or hides both LHS and RHS.

I think this issue resolves the last checkbox of #229 (please confirm @tfiedor), now that stats and metadata may be fully user-defined, including various aggregation and comparison functions, etc.

The profile stats format now defines the ordering before the unit.
Also, when parsing the stats and imported profiles, some leading
and trailing whitespaces are now being stripped.
Profile metadata may be specified in the import CLI now.

A proper internal representation of profile metadata has been
implemented so that metadata may have a tooltip information
associated with them.
Relative paths (to csv files, machine info file, profiles, ...)
are now always prepended with the import-dir path, if provided.

Absolute paths ignore the import-dir path and are kept as is.
Profile specification, stats and metadata now collapse / show
both LHS and RHS at the same time.

Multiple import entries may be specified; an import entry is either a profile entry

'profile_path[,<exit code>[,<stat value>]+]'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? And can you maybe write an example how his is run? That's better readable then nested parenthesized specification.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parentheses represent optional parts of the import entries. This change was done so that (a) CSV and CLI specification of profiles to import is unified, i.e., it is now possible to specify exactly the same import parameters using both CSV files and CLI, and (b) at least exit code may be specified on CLI alongside the profiles to import. However, the interface is backwards-compatible and simple profile paths, e.g., import.stack.gz may still be specified. An example input containing both exit code and some stats values (corresponding to some specified stats headers defined in option --stats-headers) is, e.g., 'import.stack.gz,0,18511.379883,367'.

@JiriPavela JiriPavela linked an issue Sep 25, 2024 that may be closed by this pull request
6 tasks
@JiriPavela JiriPavela merged commit 752491d into Perfexionists:devel Sep 25, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the visualizations of differences
2 participants