Re: [RFC 0/6] scripts: Rewrite simpletrace printer in Rust

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Cc'ing a few more Rust integration reviewers :)

On 27/5/24 10:14, Zhao Liu wrote:
> Hi maintainers and list,
> 
> This RFC series attempts to re-implement simpletrace.py with Rust, which
> is the 1st task of Paolo's GSoC 2024 proposal.
> 
> There are two motivations for this work:
> 1. This is an open chance to discuss how to integrate Rust into QEMU.
> 2. Rust delivers faster parsing.
> 
> 
> Introduction
> ============
> 
> Code framework
> --------------
> 
> I choose "cargo" to organize the code, because the current
> implementation depends on external crates (Rust's library), such as
> "backtrace" for getting frameinfo, "clap" for parsing the cli, "rex" for
> regular matching, and so on. (Meson's support for external crates is
> still incomplete. [2])
> 
> The simpletrace-rust created in this series is not yet integrated into
> the QEMU compilation chain, so it can only be compiled independently, e.g.
> under ./scripts/simpletrace/, compile it be:
> 
>      cargo build --release
> 
> The code tree for the entire simpletrace-rust is as follows:
> 
> $ script/simpletrace-rust .
> .
> ├── Cargo.toml
> └── src
>      └── main.rs   // The simpletrace logic (similar to simpletrace.py).
>      └── trace.rs  // The Argument and Event abstraction (refer to
>                    // tracetool/__init__.py).
> 
> My question about meson v.s. cargo, I put it at the end of the cover
> letter (the section "Opens on Rust Support").
> 
> The following two sections are lessons I've learned from this Rust
> practice.
> 
> 
> Performance
> -----------
> 
> I did the performance comparison using the rust-simpletrace prototype with
> the python one:
> 
> * On the i7-10700 CPU @ 2.90GHz machine, parsing and outputting a 35M
> trace binary file for 10 times on each item:
> 
>                        AVE (ms)       Rust v.s. Python
> Rust   (stdout)       12687.16            114.46%
> Python (stdout)       14521.85
> 
> Rust   (file)          1422.44            264.99%
> Python (file)          3769.37
> 
> - The "stdout" lines represent output to the screen.
> - The "file" lines represent output to a file (via "> file").
> 
> This Rust version contains some optimizations (including print, regular
> matching, etc.), but there should be plenty of room for optimization.
> 
> The current performance bottleneck is the reading binary trace file,
> since I am parsing headers and event payloads one after the other, so
> that the IO read overhead accounts for 33%, which can be further
> optimized in the future.
> 
> 
> Security
> --------
> 
> This is an example.
> 
> Rust is very strict about type-checking, and it found timestamp reversal
> issue in simpletrace-rust [3] (sorry, haven't gotten around to digging
> deeper with more time)...in this RFC, I workingaround it by allowing
> negative values. And the python version, just silently covered this
> issue up.
> 
> 
> Opens on Rust Support
> =====================
> 
> Meson v.s. Cargo
> ----------------
> 
> The first question is whether all Rust code (including under scripts)
> must be integrated into meson?
> 
> If so, because of [2] then I have to discard the external crates and
> build some more Rust wheels of my own to replace the previous external
> crates.
> 
> For the main part of the QEMU code, I think the answer must be Yes, but
> for the tools in the scripts directory, would it be possible to allow
> the use of cargo to build small tools/program for flexibility and
> migrate to meson later (as meson's support for rust becomes more
> mature)?
> 
> 
> External crates
> ---------------
> 
> This is an additional question that naturally follows from the above
> question, do we have requirements for Rust's external crate? Is only std
> allowed?
> 
> Welcome your feedback!
> 
> 
> [1]: https://wiki.qemu.org/Google_Summer_of_Code_2024
> [2]: https://github.com/mesonbuild/meson/issues/2173
> [3]: https://lore.kernel.org/qemu-devel/20240509134712.GA515599@fedora.redhat.com/
> 
> Thanks and Best Regards,
> Zhao
> 
> ---
> Zhao Liu (6):
>    scripts/simpletrace-rust: Add the basic cargo framework
>    scripts/simpletrace-rust: Support Event & Arguments in trace module
>    scripts/simpletrace-rust: Add helpers to parse trace file
>    scripts/simpletrace-rust: Parse and check trace recode file
>    scripts/simpletrace-rust: Format simple trace output
>    docs/tracing: Add simpletrace-rust section
> 
>   docs/devel/tracing.rst                 |  35 ++
>   scripts/simpletrace-rust/.gitignore    |   1 +
>   scripts/simpletrace-rust/.rustfmt.toml |   9 +
>   scripts/simpletrace-rust/Cargo.lock    | 370 +++++++++++++++
>   scripts/simpletrace-rust/Cargo.toml    |  17 +
>   scripts/simpletrace-rust/src/main.rs   | 633 +++++++++++++++++++++++++
>   scripts/simpletrace-rust/src/trace.rs  | 339 +++++++++++++
>   7 files changed, 1404 insertions(+)
>   create mode 100644 scripts/simpletrace-rust/.gitignore
>   create mode 100644 scripts/simpletrace-rust/.rustfmt.toml
>   create mode 100644 scripts/simpletrace-rust/Cargo.lock
>   create mode 100644 scripts/simpletrace-rust/Cargo.toml
>   create mode 100644 scripts/simpletrace-rust/src/main.rs
>   create mode 100644 scripts/simpletrace-rust/src/trace.rs
>