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

[Zhao Liu <zhao1.liu@intel.com>]

Hi Stefan,

On Mon, May 27, 2024 at 03:59:44PM -0400, Stefan Hajnoczi wrote:
> Date: Mon, 27 May 2024 15:59:44 -0400
> From: Stefan Hajnoczi <stefanha@redhat.com>
> Subject: Re: [RFC 0/6] scripts: Rewrite simpletrace printer in Rust
> 
> On Mon, May 27, 2024 at 04:14:15PM +0800, 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
> 
> Please make sure it's built by .gitlab-ci.d/ so that the continuous
> integration system prevents bitrot. You can add a job that runs the
> cargo build.

Thanks! I'll do this.

> > 
> > 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.
> 
> Performance will become more important when large amounts of TCG data is
> captured, as described in the project idea:
> https://wiki.qemu.org/Internships/ProjectIdeas/TCGBinaryTracing
> 
> While I can't think of a time in the past where simpletrace.py's
> performance bothered me, improving performance is still welcome. Just
> don't spend too much time on performance (and making the code more
> complex) unless there is a real need.

Yes, I agree that it shouldn't be over-optimized.

The logic in the current Rust version is pretty much a carbon copy of
the Python version, without additional complex logic introduced, but the
improvements in x2.6 were obtained by optimizing IO:

* reading the event configuration file, where I called the buffered
  interface,
* and the output formatted trace log, which I output all via std_out.lock()
  followed by write_all().

So, just the simple tweak of the interfaces brings much benefits. :-)

> > 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)?
> 
> I have not seen a satisfying way to natively build Rust code using
> meson. I remember reading about a tool that converts Cargo.toml files to
> meson wrap files or something similar. That still doesn't feel great
> because upstream works with Cargo and duplicating build information in
> meson is a drag.
> 
> Calling cargo from meson is not ideal either, but it works and avoids
> duplicating build information. This is the approach I would use for now
> unless someone can point to an example of native Rust support in meson
> that is clean.
> 
> Here is how libblkio calls cargo from meson:
> https://gitlab.com/libblkio/libblkio/-/blob/main/src/meson.build
> https://gitlab.com/libblkio/libblkio/-/blob/main/src/cargo-build.sh

Many thanks! This is a good example and I'll try to build similarly to
it.

> > 
> > 
> > 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?
> 
> There is no policy. My suggestion:
> 
> If you need a third-party crate then it's okay to use it, but try to
> minimize dependencies. Avoid adding dependening for niceties that are
> not strictly needed. Third-party crates are a burden for package
> maintainers and anyone building from source. They increase the risk that
> the code will fail to build. They can also be a security risk.

Thanks for the suggestion, that's clear to me, I'll try to control the
third party dependencies.

> > 
> > Welcome your feedback!
> 
> It would be easier to give feedback if you implement some examples of
> TCG binary tracing before rewriting simpletrace.py. It's unclear to me
> why simpletrace needs to be rewritten at this point. If you are
> extending the simpletrace file format in ways that are not suitable for
> Python or can demonstrate that Python performance is a problem, then
> focussing on a Rust simpletrace implementation is more convincing.
> 
> Could you use simpletrace.py to develop TCG binary tracing first?

Yes, I can. :-)

Rewriting in Rust does sound duplicative, but I wonder if this could be
viewed as an open opportunity to add Rust support for QEMU, looking at
the scripts directory to start exploring Rust support/build would be
a relatively easy place to start.

I think the exploration of Rust's build of the simpletrace tool under
scripts parts can help with subsequent work on supporting Rust in other
QEMU core parts.

From this point, may I ask your opinion?

Maybe later, Rust-simpletrace and python-simpletrace can differ, e.g.
the former goes for performance and the latter for scalability.

Thanks,
Zhao