[libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in ChromeOS
Cheng-Hao Yang
chenghaoyang at chromium.org
Thu Aug 22 17:07:39 CEST 2024
Gentle ping: perfetto is an important feature in CrOS to do the debugging.
It'd be nice to have it supported earlier.
Thanks!
On Tue, Jul 30, 2024 at 3:30 PM Cheng-Hao Yang <chenghaoyang at chromium.org>
wrote:
> 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/20240822/ca10b97c/attachment.htm>
More information about the libcamera-devel
mailing list