mbox series

[v4,00/14] simpletrace: refactor and general improvements

Message ID 20230823085429.20519-1-mads@ynddal.dk (mailing list archive)
Headers show
Series simpletrace: refactor and general improvements | expand

Message

Mads Ynddal Aug. 23, 2023, 8:54 a.m. UTC
From: Mads Ynddal <m.ynddal@samsung.com>

I wanted to use simpletrace.py for an internal project, so I tried to update
and polish the code. Some of the commits resolve specific issues, while some
are more subjective.

I've tried to divide it into commits so we can discuss the
individual changes, and I'm ready to pull things out, if it isn't needed.

v4:
 * Added missing Analyzer2 to __all__
 * Rebased with master
v3:
 * Added __all__ with public interface
 * Added comment about magic numbers and structs from Stefan Hajnoczi
 * Reintroduced old interface for process, run and Analyzer
 * Added comment about Python 3.6 in ref. to getfullargspec
 * process now accepts events as file-like objects
 * Updated context-manager code for Analyzer
 * Moved logic of event processing to Analyzer class
 * Moved logic of process into _process function
 * Added new Analyzer2 class with kwarg event-processing
 * Reverted changes to process-call in scripts/analyse-locks-simpletrace.py
v2:
 * Added myself as maintainer of simpletrace.py
 * Improve docstring on `process`
 * Changed call to `process` in scripts/analyse-locks-simpletrace.py to reflect new argument types
 * Replaced `iteritems()` with `items()` in scripts/analyse-locks-simpletrace.py to support Python 3

Mads Ynddal (14):
  simpletrace: add __all__ to define public interface
  simpletrace: annotate magic constants from QEMU code
  simpletrace: improve parsing of sys.argv; fix files never closed.
  simpletrace: changed naming of edict and idtoname to improve
    readability
  simpletrace: update code for Python 3.11
  simpletrace: improved error handling on struct unpack
  simpletrace: define exception and add handling
  simpletrace: made Analyzer into context-manager
  simpletrace: refactor to separate responsibilities
  simpletrace: move logic of process into internal function
  simpletrace: move event processing to Analyzer class
  simpletrace: added simplified Analyzer2 class
  MAINTAINERS: add maintainer of simpletrace.py
  scripts/analyse-locks-simpletrace.py: changed iteritems() to items()

 MAINTAINERS                          |   6 +
 scripts/analyse-locks-simpletrace.py |   2 +-
 scripts/simpletrace-benchmark.zip    | Bin 0 -> 4809 bytes
 scripts/simpletrace.py               | 362 ++++++++++++++++++---------
 4 files changed, 247 insertions(+), 123 deletions(-)
 create mode 100644 scripts/simpletrace-benchmark.zip

Comments

Mads Ynddal Sept. 5, 2023, 10:46 a.m. UTC | #1
> On 23 Aug 2023, at 10.54, Mads Ynddal <mads@ynddal.dk> wrote:
> 
> From: Mads Ynddal <m.ynddal@samsung.com>
> 
> I wanted to use simpletrace.py for an internal project, so I tried to update
> and polish the code. Some of the commits resolve specific issues, while some
> are more subjective.
> 
> I've tried to divide it into commits so we can discuss the
> individual changes, and I'm ready to pull things out, if it isn't needed.
> 
> v4:
> * Added missing Analyzer2 to __all__
> * Rebased with master
> v3:
> * Added __all__ with public interface
> * Added comment about magic numbers and structs from Stefan Hajnoczi
> * Reintroduced old interface for process, run and Analyzer
> * Added comment about Python 3.6 in ref. to getfullargspec
> * process now accepts events as file-like objects
> * Updated context-manager code for Analyzer
> * Moved logic of event processing to Analyzer class
> * Moved logic of process into _process function
> * Added new Analyzer2 class with kwarg event-processing
> * Reverted changes to process-call in scripts/analyse-locks-simpletrace.py
> v2:
> * Added myself as maintainer of simpletrace.py
> * Improve docstring on `process`
> * Changed call to `process` in scripts/analyse-locks-simpletrace.py to reflect new argument types
> * Replaced `iteritems()` with `items()` in scripts/analyse-locks-simpletrace.py to support Python 3
> 
> Mads Ynddal (14):
>  simpletrace: add __all__ to define public interface
>  simpletrace: annotate magic constants from QEMU code
>  simpletrace: improve parsing of sys.argv; fix files never closed.
>  simpletrace: changed naming of edict and idtoname to improve
>    readability
>  simpletrace: update code for Python 3.11
>  simpletrace: improved error handling on struct unpack
>  simpletrace: define exception and add handling
>  simpletrace: made Analyzer into context-manager
>  simpletrace: refactor to separate responsibilities
>  simpletrace: move logic of process into internal function
>  simpletrace: move event processing to Analyzer class
>  simpletrace: added simplified Analyzer2 class
>  MAINTAINERS: add maintainer of simpletrace.py
>  scripts/analyse-locks-simpletrace.py: changed iteritems() to items()
> 
> MAINTAINERS                          |   6 +
> scripts/analyse-locks-simpletrace.py |   2 +-
> scripts/simpletrace-benchmark.zip    | Bin 0 -> 4809 bytes
> scripts/simpletrace.py               | 362 ++++++++++++++++++---------
> 4 files changed, 247 insertions(+), 123 deletions(-)
> create mode 100644 scripts/simpletrace-benchmark.zip
> 
> -- 
> 2.38.1
> 

Ping :)
Stefan Hajnoczi Sept. 20, 2023, 8:39 p.m. UTC | #2
On Wed, Aug 23, 2023 at 10:54:15AM +0200, Mads Ynddal wrote:
> From: Mads Ynddal <m.ynddal@samsung.com>
> 
> I wanted to use simpletrace.py for an internal project, so I tried to update
> and polish the code. Some of the commits resolve specific issues, while some
> are more subjective.

Hi Mads,
Apologies for the very late review. I'm happy with this series except
for the zip file and unused Formatter2 class. Please drop them and
resend.

Thanks,
Stefan

> 
> I've tried to divide it into commits so we can discuss the
> individual changes, and I'm ready to pull things out, if it isn't needed.
> 
> v4:
>  * Added missing Analyzer2 to __all__
>  * Rebased with master
> v3:
>  * Added __all__ with public interface
>  * Added comment about magic numbers and structs from Stefan Hajnoczi
>  * Reintroduced old interface for process, run and Analyzer
>  * Added comment about Python 3.6 in ref. to getfullargspec
>  * process now accepts events as file-like objects
>  * Updated context-manager code for Analyzer
>  * Moved logic of event processing to Analyzer class
>  * Moved logic of process into _process function
>  * Added new Analyzer2 class with kwarg event-processing
>  * Reverted changes to process-call in scripts/analyse-locks-simpletrace.py
> v2:
>  * Added myself as maintainer of simpletrace.py
>  * Improve docstring on `process`
>  * Changed call to `process` in scripts/analyse-locks-simpletrace.py to reflect new argument types
>  * Replaced `iteritems()` with `items()` in scripts/analyse-locks-simpletrace.py to support Python 3
> 
> Mads Ynddal (14):
>   simpletrace: add __all__ to define public interface
>   simpletrace: annotate magic constants from QEMU code
>   simpletrace: improve parsing of sys.argv; fix files never closed.
>   simpletrace: changed naming of edict and idtoname to improve
>     readability
>   simpletrace: update code for Python 3.11
>   simpletrace: improved error handling on struct unpack
>   simpletrace: define exception and add handling
>   simpletrace: made Analyzer into context-manager
>   simpletrace: refactor to separate responsibilities
>   simpletrace: move logic of process into internal function
>   simpletrace: move event processing to Analyzer class
>   simpletrace: added simplified Analyzer2 class
>   MAINTAINERS: add maintainer of simpletrace.py
>   scripts/analyse-locks-simpletrace.py: changed iteritems() to items()
> 
>  MAINTAINERS                          |   6 +
>  scripts/analyse-locks-simpletrace.py |   2 +-
>  scripts/simpletrace-benchmark.zip    | Bin 0 -> 4809 bytes
>  scripts/simpletrace.py               | 362 ++++++++++++++++++---------
>  4 files changed, 247 insertions(+), 123 deletions(-)
>  create mode 100644 scripts/simpletrace-benchmark.zip
> 
> -- 
> 2.38.1
>