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

18040 diff mbox series

As ChromeOS is using perfetto (project named as CrOSetto). When ChromeOS
uses libcamera, it should use perfetto to collect traces instead of
lttng.

Signed-off-by: Harvey Yang <chenghaoyang@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

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@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.

> +	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)

> +
> +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).

[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
>