[libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in ChromeOS
Cheng-Hao Yang
chenghaoyang at chromium.org
Tue Jul 30 15:30:38 CEST 2024
Hi Paul,
Sorry for the late reply. I've been working on mtkisp7's migration :p
On Tue, Dec 27, 2022 at 12:09 PM Paul Elder <paul.elder at ideasonboard.com>
wrote:
> Hi Harvey,
>
> On Tue, Dec 20, 2022 at 07:05:23AM +0000, Harvey Yang via libcamera-devel
> wrote:
> > As ChromeOS is using perfetto (project named as CrOSetto). When ChromeOS
> > uses libcamera, it should use perfetto to collect traces instead of
> > lttng.
>
> I'll start with the elephant in the room: we cannot have duplicate
> tracepoint definitions. There must be only one definition of the
> tracepoints, and either one or both of perfetto's and lttng's
> tracepoints must be generated from that.
>
> For instance, I've already got a series pending [1] that expands on the
> request tracepoints quite a bit; it's clearly unscalable to deal with
> two duplicate sets of tracepoints (I'm not sure why I have to defend
> deduplication, but anyway just trying to be clear about my stance).
>
> [1] https://patchwork.libcamera.org/project/libcamera/list/?series=3668
>
> More on this in the relevant section in the patch.
>
> >
> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> > ---
> > Documentation/guides/tracing.rst | 125 ++++++++++++++----
> > include/libcamera/internal/tracepoints.h.in | 38 +++++-
> > .../internal/tracepoints/meson.build | 25 ++--
> > .../internal/tracepoints/pipeline.perfetto | 10 ++
> > .../internal/tracepoints/request.perfetto | 30 +++++
> > meson.build | 7 +-
> > src/android/cros/camera3_hal.cpp | 5 +
> > src/android/cros/meson.build | 1 +
> > src/libcamera/meson.build | 14 +-
> > src/libcamera/perfetto/meson.build | 6 +
> > src/libcamera/perfetto/pipeline_perfetto.cpp | 24 ++++
> > src/libcamera/perfetto/request_perfetto.cpp | 73 ++++++++++
> > src/libcamera/tracepoints.cpp | 11 ++
> > 13 files changed, 325 insertions(+), 44 deletions(-)
> > create mode 100644
> include/libcamera/internal/tracepoints/pipeline.perfetto
> > create mode 100644
> include/libcamera/internal/tracepoints/request.perfetto
> > create mode 100644 src/libcamera/perfetto/meson.build
> > create mode 100644 src/libcamera/perfetto/pipeline_perfetto.cpp
> > create mode 100644 src/libcamera/perfetto/request_perfetto.cpp
> >
> > diff --git a/Documentation/guides/tracing.rst
> b/Documentation/guides/tracing.rst
> > index ae960d85..bd5b3453 100644
> > --- a/Documentation/guides/tracing.rst
> > +++ b/Documentation/guides/tracing.rst
> > @@ -16,33 +16,50 @@ at periodic points in time. This can be done with
> other tools such as
> > callgrind, perf, gprof, etc., without modification to the application,
> > and is out of scope for this guide.
> >
> > +Library: Perfetto vs lttng-ust
> > +------------------------------
> > +
> > +To integrate with CrOS tracing infrastructure, which uses perfetto (or
> called
>
> The first instance of "CrOS" should probably be spelled out.
>
> > +CrOSetto in CrOS), we implement the tracepoint macros (will be
> described below)
> > +with two different libraries: CrOS with perfetto, and the rest with
> lttng-ust.
> > +
> > Compiling
> > ---------
> >
> > -To compile libcamera with tracing support, it must be enabled through
> the
> > -meson ``tracing`` option. It depends on the lttng-ust library
> (available in the
> > -``liblttng-ust-dev`` package for Debian-based distributions).
> > -By default the tracing option in meson is set to ``auto``, so if
> > -liblttng is detected, it will be enabled by default. Conversely, if the
> option
> > -is set to disabled, then libcamera will be compiled without tracing
> support.
> > +To compile libcamera with tracing support in CrOS, it must be enabled
> through
>
> s/ in CrOS//
>
> > +the meson ``tracing`` option. In CrOS, it depends on the perfetto
> library,
> > +which is by default available in CrOS; In the rest, it depends on the
> lttng-ust
>
> s/;/./
>
> s/the rest/non-CrOS/
>
> > +library (available in the ``liblttng-ust-dev`` package for Debian-based
> > +distributions).
> > +By default the tracing option in meson is set to ``auto``, so if the
> library is
> > +detected, it will be enabled by default. Conversely, if the option is
> set to
> > +disabled, then libcamera will be compiled without tracing support.
> >
> > Defining tracepoints
> > --------------------
> >
> > libcamera already contains a set of tracepoints. To define additional
> > -tracepoints, create a file
> > -``include/libcamera/internal/tracepoints/{file}.tp``, where ``file`` is
> a
> > -reasonable name related to the category of tracepoints that you wish to
> > -define. For example, the tracepoints file for the Request object is
> called
> > -``request.tp``. An entry for this file must be added in
> > -``include/libcamera/internal/tracepoints/meson.build``.
> > -
> > -In this tracepoints file, define your tracepoints `as mandated by lttng
> > +tracepoints, create two files ``{file}.tp`` and ``{file}.perfetto``,
> for lttng
> > +and perfetto respectively, under
> ``include/libcamera/internal/tracepoints/``,
> > +and the perfetto implementation file
> ``src/libcamera/{file}_perfetto.cpp``,
> > +where ``file`` is a reasonable name related to the category of
> tracepoints that
> > +you wish to define. For example, the tracepoints files for the Request
> object is
> > +called ``request.tp`` and ``request.perfetto``. Entries for the files
> must be
> > +added in ``include/libcamera/internal/tracepoints/meson.build``.
> > +
> > +In the perfetto tracepoints files, declare the tracepoint functions in
> > +``{file}.perfetto``, and define them in ``{file}_perfetto.cpp``, `as
> mandated
> > +by perfetto <https://perfetto.dev/docs/instrumentation/track-events>`_.
> > +Currently the only enabled perfetto::Category is ``libcamera``. If you
> intend to
> > +use another category, remember to define it in
> > +``include/libcamera/internal/tracepoints.h.in``, and enable it when
> collecting
> > +traces.
>
> These sections will have to be fixed.
>
> > +
> > +In the lttng tracepoints file, define your tracepoints `as mandated by
> lttng
> > <https://lttng.org/man/3/lttng-ust>`_. The header boilerplate must
> *not* be
> > included (as it will conflict with the rest of our infrastructure), and
> > only the tracepoint definitions (with the ``TRACEPOINT_*`` macros)
> should be
> > included.
> > -
> > All tracepoint providers shall be ``libcamera``. According to lttng, the
> > tracepoint provider should be per-project; this is the rationale for
> this
> > decision. To group tracepoint events, we recommend using
> > @@ -68,9 +85,9 @@ Then to use the tracepoint:
> >
> > ``LIBCAMERA_TRACEPOINT({tracepoint_event}, args...)``
> >
> > -This macro must be used, as opposed to lttng's macros directly, because
> > -lttng is an optional dependency of libcamera, so the code must compile
> and run
> > -even when lttng is not present or when tracing is disabled.
> > +This macro must be used, as opposed to perfetto's or lttng's macros
> directly,
> > +because tracing support is optional of libcamera, so the code must
> compile and
>
> s/of/in/
>
> > +run even when perfetto or lttng are not present or when tracing is
> disabled.
> >
> > The tracepoint provider name, as declared in the tracepoint definition,
> is not
> > included in the parameters of the tracepoint.
> > @@ -87,21 +104,75 @@ respectively. These are the tracepoints that our
> sample analysis script
> > (see "Analyzing a trace") scans for when computing statistics on IPA
> call time.
> >
> > Using tracepoints (from an application)
> > ----------------------------------------
> > +---------------------------------------------
> >
> > As applications are not part of libcamera, but rather users of
> libcamera,
> > applications should seek their own tracing mechanisms. For ease of
> tracing
> > the application alongside tracing libcamera, it is recommended to also
> > -`use lttng <
> https://lttng.org/docs/#doc-tracing-your-own-user-application>`_.
> > +`use lttng <
> https://lttng.org/docs/#doc-tracing-your-own-user-application>`_,
> > +or `use perfetto <https://perfetto.dev/docs/instrumentation/tracing-sdk
> >`_.
> >
> > Using tracepoints (from closed-source IPA)
> > -------------------------------------------
> > +------------------------------------------------
>
> Underline length.
>
> > +
> > +Similar to applications, closed-source IPAs can simply use perfetto or
> lttng
> > +on their own, or any other tracing mechanism if desired.
> > +
> > +Collecting a perfetto trace
> > +---------------------------
> > +
> > +A trace can be collected with the following steps:
> > +
> > +1. Start `traced` if it hasn't been started.
> > +.. code-block:: bash
> > +
> > + start traced
> > +
> > +2. Start a consumer that includes “track_event” data source in the trace
> > + config, and “libcamera” category.
> > +.. code-block:: bash
> > +
> > + perfetto -c - --txt -o /tmp/perfetto-trace \
>
> (Reply to your comment in the other email) I think this is sufficient,
> as apparently the option for a config file is documented in the help
> text.
>
> > + <<EOF
> > +
> > + buffers: {
> > + size_kb: 63488
> > + fill_policy: DISCARD
> > + }
> > + buffers: {
> > + size_kb: 2048
> > + fill_policy: DISCARD
> > + }
> > + data_sources: {
> > + config {
> > + name: "track_event"
> > + track_event_config {
> > + enabled_categories: "libcamera"
> > + }
> > + }
> > + }
> > + duration_ms: 10000
> > +
> > + EOF
> > +
> > +3. Execute the libcamera behavior you intend to trace during the set
> duration
> > + (10000 ms) in the above example.
> > +
> > +4. After the consumer (cmd `perfetto`) is done, you can find the trace
> result
> > + in ``/tmp/perfetto-trace``.
> > +
> > +Analyzing a perfetto trace
> > +--------------------------
> > +
> > +Follow the `guide <https://perfetto.dev/docs/visualization/perfetto-ui>`_
> to
> > +visualize the trace.
> >
> > -Similar to applications, closed-source IPAs can simply use lttng on
> their own,
> > -or any other tracing mechanism if desired.
> > +In other words, upload the trace to `Perfetto UI <
> https://ui.perfetto.dev/>`_,
> > +where you can check the timeline of tracepoints on each process/thread.
> You can
> > +also run SQL queries to do analysis.
> >
> > -Collecting a trace
> > -------------------
> > +Collecting an lttng trace
> > +------------------------
>
> Underline length.
>
> >
> > A trace can be collected fairly simply from lttng:
> >
> > @@ -123,8 +194,8 @@ viewed by: ``lttng view -t $PATH_TO_TRACE``, where
> ``$PATH_TO_TRACE`` is the
> > path that was printed when the session was created. This is the same
> path that
> > is used when analyzing traces programatically, as described in the next
> section.
> >
> > -Analyzing a trace
> > ------------------
> > +Analyzing a lttng trace
>
> s/a/an/
>
> > +-----------------------
> >
> > As mentioned above, while an lttng tracing session exists and the trace
> is not
> > running, the trace output can be viewed as text by ``lttng view``.
> > diff --git a/include/libcamera/internal/tracepoints.h.in
> b/include/libcamera/internal/tracepoints.h.in
> > index d0fc1365..b6c4abea 100644
> > --- a/include/libcamera/internal/tracepoints.h.in
> > +++ b/include/libcamera/internal/tracepoints.h.in
> > @@ -9,7 +9,24 @@
> > #ifndef __LIBCAMERA_INTERNAL_TRACEPOINTS_H__
> > #define __LIBCAMERA_INTERNAL_TRACEPOINTS_H__
> >
> > -#if HAVE_TRACING
> > +#if HAVE_PERFETTO
> > +
> > +#include <perfetto/perfetto.h>
> > +
> > +PERFETTO_DEFINE_CATEGORIES(
> > + perfetto::Category("libcamera")
> > + .SetDescription("Events from libcamera"));
> > +
> > +#define LIBCAMERA_TRACEPOINT(t_name, ...) \
> > +LIBCAMERA_TRACE_EVENT_##t_name(__VA_ARGS__)
> > +
> > +#define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \
> > +LIBCAMERA_TRACE_EVENT_ipa_call_begin(#pipe, #func)
> > +
> > +#define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \
> > +LIBCAMERA_TRACE_EVENT_ipa_call_end(#pipe, #func)
> > +
> > +#elif HAVE_LTTNG /* !HAVE_PERFETTO */
> > #define LIBCAMERA_TRACEPOINT(...) tracepoint(libcamera, __VA_ARGS__)
> >
> > #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \
> > @@ -18,7 +35,7 @@ tracepoint(libcamera, ipa_call_begin, #pipe, #func)
> > #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \
> > tracepoint(libcamera, ipa_call_end, #pipe, #func)
> >
> > -#else
> > +#else /* !HAVE_PERFETTO && !HAVE_LTTNG */
> >
> > namespace {
> >
> > @@ -34,12 +51,17 @@ inline void unused([[maybe_unused]] Args&& ...args)
> > #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func)
> > #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func)
> >
> > -#endif /* HAVE_TRACING */
> > +#endif /* HAVE_PERFETTO */
> >
> > #endif /* __LIBCAMERA_INTERNAL_TRACEPOINTS_H__ */
> >
> > +#if HAVE_PERFETTO || HAVE_LTTNG
> > +
> > +#if HAVE_PERFETTO
> >
> > -#if HAVE_TRACING
> > +#include <perfetto/perfetto.h>
> > +
> > +#else /* HAVE_LTTNG */
> >
> > #undef TRACEPOINT_PROVIDER
> > #define TRACEPOINT_PROVIDER libcamera
> > @@ -47,15 +69,21 @@ inline void unused([[maybe_unused]] Args&& ...args)
> > #undef TRACEPOINT_INCLUDE
> > #define TRACEPOINT_INCLUDE "{{path}}"
> >
> > +#endif /* HAVE_PERFETTO */
> > +
> > #if !defined(INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H) ||
> defined(TRACEPOINT_HEADER_MULTI_READ)
> > #define INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H
>
> I'm pretty sure #endif /* HAVE_PERFETTO */ should come here (unless
> perfetto also needs this? Does it?).
>
> >
> > +#if HAVE_LTTNG
> > #include <lttng/tracepoint.h>
> > +#endif /* HAVE_LTTNG */
> >
> > {{source}}
> >
> > #endif /* INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H */
>
> Same for this.
>
> I wonder if it's easier just to have a top-level `if HAVE_LTTNG and if
> HAVE_PERFETTO` and just duplicate {{source}}. It's only one line that's
> duplicated anyway.
>
> >
> > +#if HAVE_LTTNG
> > #include <lttng/tracepoint-event.h>
> > +#endif /* HAVE_LTTNG */
> >
> > -#endif /* HAVE_TRACING */
> > +#endif /* HAVE_PERFETTO || HAVE_LTTNG */
> > diff --git a/include/libcamera/internal/tracepoints/meson.build
> b/include/libcamera/internal/tracepoints/meson.build
> > index d9b2fca5..ff5aece6 100644
> > --- a/include/libcamera/internal/tracepoints/meson.build
> > +++ b/include/libcamera/internal/tracepoints/meson.build
> > @@ -1,12 +1,19 @@
> > # SPDX-License-Identifier: CC0-1.0
> >
> > -# enum files must go first
> > -tracepoint_files = files([
> > - 'buffer_enums.tp',
> > - 'request_enums.tp',
> > -])
> > +if get_option('android').enabled() and get_option('android_platform')
> == 'cros'
> > + tracepoint_files = files([
> > + 'pipeline.perfetto',
> > + 'request.perfetto',
> > + ])
> > +else
> > + # enum files must go first
> > + tracepoint_files = files([
> > + 'buffer_enums.tp',
> > + 'request_enums.tp',
> > + ])
> >
> > -tracepoint_files += files([
> > - 'pipeline.tp',
> > - 'request.tp',
> > -])
> > + tracepoint_files += files([
> > + 'pipeline.tp',
> > + 'request.tp',
> > + ])
> > +endif
>
> And this is going to need a bit more moving parts for code generation...
>
> > diff --git a/include/libcamera/internal/tracepoints/pipeline.perfetto
> b/include/libcamera/internal/tracepoints/pipeline.perfetto
> > new file mode 100644
> > index 00000000..5f45295e
> > --- /dev/null
> > +++ b/include/libcamera/internal/tracepoints/pipeline.perfetto
> > @@ -0,0 +1,10 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2022, Google Inc.
> > + *
> > + * pipeline.tp - Tracepoints for pipelines
> > + */
> > +
> > +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char
> *func);
> > +
> > +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char
> *func);
>
> Basically, we already have definitions for these tracepoints in
> include/libcamera/internal/tracepoints/pipeline.tp. It's obviously not a
> good idea to have duplicate definitions.
>
> So we either need to use lttng's tracepoints definitions to generate
> perfetto's here, or we create a new language from which we generate
> tracepoints for both. imo the latter is overkill (and costs a lot more
> effort) so probably the former is better.
>
> I'll use the request tracepoints as examples below just because they
> have a bit more beef than these pipeline tracepoints which are just two
> strings.
>
> > diff --git a/include/libcamera/internal/tracepoints/request.perfetto
> b/include/libcamera/internal/tracepoints/request.perfetto
> > new file mode 100644
> > index 00000000..fd6a42a4
> > --- /dev/null
> > +++ b/include/libcamera/internal/tracepoints/request.perfetto
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2022, Google Inc.
> > + *
> > + * request.tp - Tracepoints for the request object
> > + */
> > +
> > +#include <libcamera/internal/request.h>
> > +
> > +#include <libcamera/framebuffer.h>
> > +
> > +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req);
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req);
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req);
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req);
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req);
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request
> *req);
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private
> *req);
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private
> *req);
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(
> > + libcamera::Request::Private *req,
> > + libcamera::FrameBuffer * buf);
> > diff --git a/meson.build b/meson.build
> > index e4c0be16..79514b5b 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -156,7 +156,11 @@ libcamera_includes = include_directories('include')
> > py_modules = []
> >
> > # Libraries used by multiple components
> > -liblttng = dependency('lttng-ust', required : get_option('tracing'))
> > +libperfetto = dependency('perfetto', required :
> get_option('tracing').enabled()
> > + and get_option('android').enabled()
> > + and get_option('android_platform') == 'cros')
> > +liblttng = cc.find_library('lttng-ust', required :
> get_option('tracing').enabled()
> > + and not libperfetto.found())
> >
> > # Pipeline handlers
> > #
> > @@ -208,6 +212,7 @@ py_mod.find_installation('python3', modules:
> py_modules)
> > summary({
> > 'Enabled pipelines': pipelines,
> > 'Enabled IPA modules': enabled_ipa_modules,
> > + 'Perfetto support': perfetto_enabled,
> > 'Tracing support': tracing_enabled,
> > 'Android support': android_enabled,
> > 'GStreamer support': gst_enabled,
> > diff --git a/src/android/cros/camera3_hal.cpp
> b/src/android/cros/camera3_hal.cpp
> > index fb863b5f..2fbd7f14 100644
> > --- a/src/android/cros/camera3_hal.cpp
> > +++ b/src/android/cros/camera3_hal.cpp
> > @@ -7,10 +7,15 @@
> >
> > #include <cros-camera/cros_camera_hal.h>
> >
> > +#include "libcamera/internal/tracepoints.h"
> > #include "../camera_hal_manager.h"
> >
> > static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken
> *token)
> > {
> > + perfetto::TracingInitArgs args;
> > + args.backends |= perfetto::kSystemBackend;
> > + perfetto::Tracing::Initialize(args);
> > + perfetto::TrackEvent::Register();
> > }
> >
> > static void tear_down()
> > diff --git a/src/android/cros/meson.build b/src/android/cros/meson.build
> > index 35995dd8..68f2bd9e 100644
> > --- a/src/android/cros/meson.build
> > +++ b/src/android/cros/meson.build
> > @@ -9,5 +9,6 @@ android_hal_sources += files([
> > ])
> >
> > android_deps += dependency('libcros_camera')
> > +android_deps += dependency('perfetto')
> >
> > android_cpp_args += ['-DOS_CHROMEOS']
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 0494e808..ba8bf26e 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -62,6 +62,7 @@ libthreads = dependency('threads')
> >
> > subdir('base')
> > subdir('ipa')
> > +subdir('perfetto')
> > subdir('pipeline')
> > subdir('proxy')
> >
> > @@ -89,11 +90,19 @@ if not libcrypto.found()
> > warning('Neither gnutls nor libcrypto found, all IPA modules will
> be isolated')
> > endif
> >
> > -if liblttng.found()
> > +if libperfetto.found()
> > + perfetto_enabled = true
> > + tracing_enabled = false
> > + config_h.set('HAVE_PERFETTO', 1)
> > + libcamera_sources += perfetto_sources
> > + libcamera_sources += files(['tracepoints.cpp'])
> > +elif liblttng.found()
> > + perfetto_enabled = false
> > tracing_enabled = true
> > - config_h.set('HAVE_TRACING', 1)
> > + config_h.set('HAVE_LTTNG', 1)
> > libcamera_sources += files(['tracepoints.cpp'])
> > else
> > + perfetto_enabled = false
> > tracing_enabled = false
> > endif
> >
> > @@ -154,6 +163,7 @@ libcamera_deps = [
> > libcrypto,
> > libdl,
> > liblttng,
> > + libperfetto,
> > libudev,
> > libyaml,
> > ]
> > diff --git a/src/libcamera/perfetto/meson.build
> b/src/libcamera/perfetto/meson.build
> > new file mode 100644
> > index 00000000..119a1ad7
> > --- /dev/null
> > +++ b/src/libcamera/perfetto/meson.build
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +perfetto_sources = files([
> > + 'pipeline_perfetto.cpp',
> > + 'request_perfetto.cpp',
> > +])
> > diff --git a/src/libcamera/perfetto/pipeline_perfetto.cpp
> b/src/libcamera/perfetto/pipeline_perfetto.cpp
> > new file mode 100644
> > index 00000000..07b82ffd
> > --- /dev/null
> > +++ b/src/libcamera/perfetto/pipeline_perfetto.cpp
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2022, Google Inc.
> > + *
> > + * pipeline.tp - Tracepoints for pipelines
> > + */
> > +
> > +#include "libcamera/internal/tracepoints.h"
> > +
> > +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char
> *func)
> > +{
> > + // TODO: Consider TRACE_EVENT_BEGIN
>
> From what I've read, I think TRACE_EVENT_BEGIN is better.
>
> In the perfetto tracepoints generator we could automatically detect
> _begin and _end suffixes and convert those to TRACE_EVENT_BEGIN and
> TRACE_EVENT_END.
>
>
We might need to add an argument to set the track id, or else Perfetto
library doesn't know which `begin` the `end` belongs to.
> > + TRACE_EVENT("libcamera", "ipa_call_begin",
> > + "pipeline_name", pipe,
> > + "function_name", func);
> > +}
> > +
> > +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char
> *func)
> > +{
> > + // TODO: Consider TRACE_EVENT_END
> > + TRACE_EVENT("libcamera", "ipa_call_end",
> > + "pipeline_name", pipe,
> > + "function_name", func);
> > +}
> > diff --git a/src/libcamera/perfetto/request_perfetto.cpp
> b/src/libcamera/perfetto/request_perfetto.cpp
> > new file mode 100644
> > index 00000000..2cfff28e
> > --- /dev/null
> > +++ b/src/libcamera/perfetto/request_perfetto.cpp
> > @@ -0,0 +1,73 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2022, Google Inc.
> > + *
> > + * request.tp - Tracepoints for the request object
> > + */
> > +
> > +#include <libcamera/framebuffer.h>
> > +
> > +#include "libcamera/internal/request.h"
> > +#include "libcamera/internal/tracepoints.h"
> > +
> > +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req)
> > +{
> > + TRACE_EVENT("libcamera", "request",
> > + "request_ptr", reinterpret_cast<uintptr_t>(req),
> > + "cookie", req->cookie(),
> > + "status", req->status()); // TODO
>
> (What is this TODO for?)
>
> > +}
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req)
> > +{
> > + TRACE_EVENT("libcamera", "request_construct",
> > + "request_ptr", reinterpret_cast<uintptr_t>(req));
> > +}
>
> 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].
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
> +
> > +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req)
> > +{
> > + TRACE_EVENT("libcamera", "request_destroy",
> > + "request_ptr", reinterpret_cast<uintptr_t>(req));
> > +}
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req)
> > +{
> > + TRACE_EVENT("libcamera", "request_reuse",
> > + "request_ptr", reinterpret_cast<uintptr_t>(req));
> > +}
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req)
> > +{
> > + TRACE_EVENT("libcamera", "request_queue",
> > + "request_ptr", reinterpret_cast<uintptr_t>(req));
> > +}
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request *req)
> > +{
> > + TRACE_EVENT("libcamera", "request_device_queue",
> > + "request_ptr", reinterpret_cast<uintptr_t>(req));
> > +}
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private
> *req)
> > +{
> > + TRACE_EVENT("libcamera", "request_complete",
> > + "request_private_ptr",
> reinterpret_cast<uintptr_t>(req));
> > +}
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private
> *req)
> > +{
> > + TRACE_EVENT("libcamera", "request_cancel",
> > + "request_private_ptr",
> reinterpret_cast<uintptr_t>(req));
> > +}
> > +
> > +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(
> > + libcamera::Request::Private *req,
> > + libcamera::FrameBuffer *buf)
> > +{
> > + 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(), //
> TODO
> > + "buffer_ptr", reinterpret_cast<uintptr_t>(buf),
> > + "buffer_status", buf->metadata().status); // TODO
> > +}
>
> Alright, I'm going to use this as an example.
>
> First, the tracepoint definition. In lttng we have:
>
> TRACEPOINT_EVENT(
> libcamera,
> request_complete_buffer,
> TP_ARGS(
> libcamera::Request::Private *, req,
> libcamera::FrameBuffer *, buf
> ),
> TP_FIELDS(
> ctf_integer_hex(uintptr_t, request,
> reinterpret_cast<uintptr_t>(req))
> ctf_integer(uint64_t, cookie,
> req->_o<libcamera::Request>()->cookie())
> ctf_integer(int, status,
> req->_o<libcamera::Request>()->status())
> ctf_integer_hex(uintptr_t, buffer,
> reinterpret_cast<uintptr_t>(buf))
> ctf_enum(libcamera, buffer_status, uint32_t, buf_status,
> buf->metadata().status)
> )
> )
>
> And to raise (idk the right verb) the tracepoint, we use
> tracepoint(libcamera, request_complete_buffer, request_ptr, buffer_ptr).
> In perfetto as far as I understand, it's raised using TRACE_EVENT(...)
> and you have to specify everything that lttng wraps away in TP_FIELDS in
> the tracepoint definition itself. For this reason, I think it's good to
> have the intermediary functions.
>
> The idea is to generate both the perfetto tracepoint definition and the
> intermediary function from the lttng definition. I've studied the
> perfetto tracepoint definition docs a bit more than last time, and I
> think that going from lttng's definitions to perfetto's is better.
>
> I was going to walk through an example, but I'm not sure how because it
> seems pretty straightforward to me. We can parse the TRACEPOINT_EVENT,
> and extract the category name and tracepoint name into the function name
> and TRACE_EVENT parameters. TP_ARGS also can get parsed and copied into
> the function parameters almost as-is. TP_FIELDS needs a bit of
> transformation, but it's just extracting the field name and
> transformation, and then adding an extra cast for the type. ctf_enum
> would be a bit more involved though, but we have definitions in .tp
> files for the enums that map the integer values to strings, so we can
> use that. It might involve generating an intermediate file for all the
> enums, or I guess we could just generate enum definitions and embed that
> in the perfetto tracepoint function definitions cpp file.
>
> For TRACEPOINT_EVENT_CLASS and TRACEPOINT_EVENT_INSTANCE there's just
> one extra level of indirection to obtain TP_FIELDS, but otherwise we
> just match it based on the tracepoint class name.
>
> As for "how", we can use either ply or a custom-made simple C parser for
> parsing, and then we'd have to make a custom code generator. If we go
> homemade C parser route it doesn't have to be too complex, as we only
> have TRACEPOINT_EVENT, TRACEPOINT_EVENT_CLASS, and
> TRACEPOINT_EVENT_INSTANCE to parse, and the complex expressions are
> simply copied. And then for the code generation part we can use jinja2.
> imo the most difficult part is plumbing it through meson, but we've done
> that before in the IPC layer, so that has examples of how to do code
> generation -> compilation via meson [2] [3] [4], as well as examples for
> jinja2 (although it's actually embedded in mojom).
>
>
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.
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.
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/20240730/a658b9f8/attachment.htm>
More information about the libcamera-devel
mailing list