Bug 467472 - Rewrite `cg_annotate` in Python
Summary: Rewrite `cg_annotate` in Python
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: cachegrind (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Nicholas Nethercote
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-17 05:14 UTC by Nick Nethercote
Modified: 2023-03-21 23:21 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Patch (85.52 KB, patch)
2023-03-17 05:15 UTC, Nick Nethercote
Details
Patch minus cg_annotate.in changes (easier to read) (23.74 KB, patch)
2023-03-17 05:16 UTC, Nick Nethercote
Details
New cg_annotate.in file (27.47 KB, text/x-python3)
2023-03-17 05:17 UTC, Nick Nethercote
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Nethercote 2023-03-17 05:14:53 UTC
Perl was a reasonable choice for `cg_annotate` in 2002, but not in 2023.
Also, the existing structure of the code is not good. These two things
make it hard to modify `cg_annotate` in any significant way.

Benefits of the change:
- Now written in a language that is (a) nice, and (b) not moribund.
- Easier to maintain, due to (a) abovementioned better language, (b)
  better code structure, and (c) better language tooling, such as
  formatters, type checkers, and linters.
- The new version is a little shorter.
- It runs about 2x faster.
- Argument handling is more standard. E.g. things like `--context 2`,
  `--auto`, `--no-auto` are supported. (The old forms that require `=`
  are still supported, though the `=yes`/`=no` forms are deprecated.)

The behaviour and output of the new version is identical for typical
uses, but there are some very minor changes for edge cases, which nobody
is likely to notice. For example:
- The file format is slightly changed: I removed support for '.'
  counts, which had the same meaning as '0'. This was a feature that
  Cachegrind never used, and the old script handled it inconsistently.
- The new version will abort on a malformed data line. The old version
  would just print a warning and continue.

The commit also adds a new test `ann3` that tests many parts of
`cg_annotate` that weren't tested previously, and tweaks the existing
`ann2` test.
Comment 1 Nick Nethercote 2023-03-17 05:15:14 UTC
Created attachment 157348 [details]
Patch
Comment 2 Nick Nethercote 2023-03-17 05:16:40 UTC
Created attachment 157349 [details]
Patch minus cg_annotate.in changes (easier to read)
Comment 3 Nick Nethercote 2023-03-17 05:17:12 UTC
Created attachment 157350 [details]
New cg_annotate.in file
Comment 4 Nick Nethercote 2023-03-21 23:21:38 UTC
I just merged a slightly improved version of the attached patch:
https://sourceware.org/git/?p=valgrind.git;a=commit;h=4650b7949ae3a41326e52ae454a9202493c41444