[libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in ChromeOS

[Laurent Pinchart]

Hi Harvey,

On Wed, Sep 11, 2024 at 06:05:18PM +0800, Cheng-Hao Yang wrote:
> On Tue, Sep 10, 2024 at 12:30 AM Paul Elder wrote:
> > Hi Harvey,
> >
> > <snip>
> >
> > >     So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments
> > >     and fields and how to obtain the fields from the args. We can
> > >     instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only
> > >     have to specify the args, and the fields are already defined and come
> > >     from the class. This reduces duplication in the tracepoint definitions.
> > >
> > >     I'm wondering from what you've implemented here that perfetto doesn't
> > >     support this? I don't see anything in the perfetto docs that suggest
> > >     that this is possible either. Although we can mitigate this issue by
> > >     generating the perfetto tracepoints :) (this is another reason why I
> > >     think we should use the lttng tracepoints definition as the main one and
> > >     generate the perfetto ones from those)
> > >
> > > I've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that
> > > there are limited features in lttng [1], and if we use the lttng tracepoints
> > > definition as the source of truth, it's hard/weird to implement some Perfetto
> > > concepts, like nested slices (lifetime of trace events) and flows [2].
> >
> > Ah ok, I see the problem.
> >
> > > It's not impossible, as you mentioned above, that we could detect the prefix
> > > and suffix of each libcamera/lttng macro. However, it's a weird code parser
> > > and generator design.
> > >
> > > [1]: https://lttng.org/man/3/lttng-ust/v2.8/
> > > [2]: https://perfetto.dev/docs/instrumentation/track-events
> >
> > <snip>
> >
> > > I agree that in the current functionalities, it's not that hard to implement
> > > a homemade C/python parser, and generate Perfetto code with jinja2.
> > > However, we not only "limit" the functionalities of Perfetto with lttng's
> > > definition, but also add another layer to be maintained.
> > > It's true that having duplication of libcamera tracepoint definitions
> > > requires more maintenance work when updating the libcamera
> > > tracepoints, while the effort to maintain the parser and code generation
> > > is also non-negligible.
> > >
> > > I also think that creating a new language from which we generate
> > > tracepoints for both is an overkill though. It seems that we don't have
> > > a perfect way to support both lttng and Perfetto...
> > >
> > > BTW, I tried lex and yacc to parse lttng macros, and tbh it's very painful
> > > that it's very hard to use lex to recognize C/C++ type as a token... So
> > > yeah if we end up deciding to take this route, a homemade parser from
> > > scratch makes more sense to me.
> > >
> > > So let me list the approaches with pros and cons:
> > > 1. Generate Perfetto code from lttng macros.
> > >   Pros: Easy to update libcamera tracepoint macros.
> > >   Cons: Add maintenance work with the additional parser and code
> > > generator; hard to implement Perfetto features that lttng doesn't have.
> >
> > This makes me wonder, if Perfetto is a superset of the features of
> > lttng, why not go the other way; use Perfetto tracepoints as the main
> > authority and then generate lttng tracepoints from those.
> >
> > What are your thoughts?
> 
> I don't think Perfetto MACROs is a superset of lttng MACROs. If we go
> with this approach, there are some features dropped (like integer_hex
> and enum, and the concept of TP_ARGS), and the code would be messy.
> Take request_complete_buffer as the example:
> 
> ```
> # Perfetto
> TRACE_EVENT("libcamera", "request_complete_buffer",
>       "request_private_ptr", reinterpret_cast<uintptr_t>(req),
>       "cookie", req->_o<libcamera::Request>()->cookie(),
>       "status", req->_o<libcamera::Request>()->status(),
>       "buffer_ptr", reinterpret_cast<uintptr_t>(buf),
>       "buffer_status", buf->metadata().status);
> ```
> 
> It'd be how perfetto implements the macro. Originally lttng has only two
> inputs: `libcamera::Request::internal *req` and
> `libcamera::FrameBuffer *buf`. Unless we're going to do complicated
> type diagnosis and simplification, we might end up with having five
> inputs as well. Also, I don't know how to indicate the usage of
> ctf_integer_hex (for `req` and `buf`) and ctf_enum (for status).
> 
> I'd still prefer the 3rd or 4th approach.

The 4th option, duplicating trace points in the libcamera sources, is
the one I like the least. The 3rd option is also something I don't like
much. It means that anyone adding trace points will need to do extra
work to support two trace systems. Testing it locally will be hard, even
for build testing. We could possibly handle the build testing with CI
(see below), even if that will be way less convenient that doing local
builds. Runtime testing is likely out of question.

I'm not comfortable with the amount of extra work this will cause to
everybody to support a Google-only trace system. I'm willing to find a
compromise though, if Google could do the necessary work needed to
package Perfetto in distributions, I could accept extra work on our side
too.

> > > 2. Creating a new language from which we generate both Perfetto
> > > and lttng macros.
> > >   Pros: Easy to update libcamera tracepoint macros.
> > >   Cons: Probably an overkill; hard to cover all features from both
> > > Perfetto and lttng.
> > >
> > > 3. As this patch implements: have duplicated libcamera tracepoint
> > > definitions.
> > >   Pros: Easy to add libcamera tracepoint macros to cover all features
> > > from Perfetto & lttng.
> > >   Cons: Developer needs to update both sides when the definition of
> > > libcamera tracepoints changes; Unused macro definitions might be
> > > adopted in both sides, if a feature is only available in one trace
> > library.
> > >
> > > 4. Add another set of libcamera tracepoints for Perfetto, or just use
> > > Perfetto macros directly in libcamera's source code.
> > >   Pros: Two isolated trace stacks that don't influence each other. All
> > > features can be implemented easily.
> > >   Cons: Duplicated trace code in source code that might lead to
> > > confusion; developers also need to update both macros when a
> > > new trace is needed.
> > >
> > > I think approach 3rd and 4th are easier to maintain in the long
> > > term. WDYT?
> > >
> > > Also, I'd like to know if you think Perfetto will be used in other linux
> > > distributions than ChromeOS or Android.
> >
> > If Perfetto gets added to the package repositories in linux
> > distributions then I think it would be used :)
> >
> > Not being able to sudo apt install perfetto is a pretty big blocker.
>
> Yeah I understand, while Perfetto is self-contained and simple to build
> in linux distros.

Do you mean by running the tools/install-build-deps script that
downloads toolchain binaries from google servers ? That doesn't really
match the philosophy of Linux distributions.

> May we consider testing it on the gitlab CI, after we
> somehow add the support?

If we add support for it I certainly would like to see it being tested
in CI, yes. After a quick look at the Perfetto build system, it looks
like adding Perfetto to the CI docker containers may not be trivial.
Someone will need to volunteer.

> > > Chinglin, please help amend anything I missed as well :)

-- 
Regards,

Laurent Pinchart