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

[Cheng-Hao Yang]

Hi Paul,

On Tue, Sep 10, 2024 at 12:30 AM Paul Elder <paul.elder at ideasonboard.com>
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.


>
> >
> > 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. May we consider testing it on the gitlab CI, after we
somehow add the support?

BR,
Harvey


>
> Thanks,
>
>
> Paul
>
> >
> > Chinglin, please help amend anything I missed as well :)
> >
> > BR,
> > Harvey
> >
> >
> >     [2] utils/ipc/meson.build
> >     [3] include/libcamera/ipa/meson.build
> >     [4] src/libcamera/ipa/meson.build
> >
> >     Would you be able to do this?
> >
> >
> >     Paul
> >
> >     > diff --git a/src/libcamera/tracepoints.cpp b/src/libcamera/
> >     tracepoints.cpp
> >     > index 0173b75a..a07ea531 100644
> >     > --- a/src/libcamera/tracepoints.cpp
> >     > +++ b/src/libcamera/tracepoints.cpp
> >     > @@ -4,7 +4,18 @@
> >     >   *
> >     >   * tracepoints.cpp - Tracepoints with lttng
> >     >   */
> >     > +
> >     > +#if HAVE_PERFETTO
> >     > +
> >     > +#include "libcamera/internal/tracepoints.h"
> >     > +
> >     > +PERFETTO_TRACK_EVENT_STATIC_STORAGE();
> >     > +
> >     > +#else
> >     > +
> >     >  #define TRACEPOINT_CREATE_PROBES
> >     >  #define TRACEPOINT_DEFINE
> >     >
> >     >  #include "libcamera/internal/tracepoints.h"
> >     > +
> >     > +#endif /* HAVE_PERFETTO */
> >     > --
> >     > 2.39.0.314.g84b9a713c41-goog
> >     >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20240911/0ccc7fa8/attachment.htm>