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

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

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

Hi Harvey,

<snip> 

>     So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments
>     and fields and how to obtain the fields from the args. We can
>     instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only
>     have to specify the args, and the fields are already defined and come
>     from the class. This reduces duplication in the tracepoint definitions.
> 
>     I'm wondering from what you've implemented here that perfetto doesn't
>     support this? I don't see anything in the perfetto docs that suggest
>     that this is possible either. Although we can mitigate this issue by
>     generating the perfetto tracepoints :) (this is another reason why I
>     think we should use the lttng tracepoints definition as the main one and
>     generate the perfetto ones from those)
> 
> 
> 
> I've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that
> there are limited features in lttng [1], and if we use the lttng tracepoints
> definition as the source of truth, it's hard/weird to implement some Perfetto
> concepts, like nested slices (lifetime of trace events) and flows [2].

Ah ok, I see the problem.

> 
> It's not impossible, as you mentioned above, that we could detect the prefix
> and suffix of each libcamera/lttng macro. However, it's a weird code parser
> and generator design.
> 
> [1]: https://lttng.org/man/3/lttng-ust/v2.8/
> [2]: https://perfetto.dev/docs/instrumentation/track-events
> 
> 

<snip> 

> 
> 
> I agree that in the current functionalities, it's not that hard to implement
> a homemade C/python parser, and generate Perfetto code with jinja2.
> However, we not only "limit" the functionalities of Perfetto with lttng's
> definition, but also add another layer to be maintained.
> It's true that having duplication of libcamera tracepoint definitions
> requires more maintenance work when updating the libcamera
> tracepoints, while the effort to maintain the parser and code generation
> is also non-negligible.
> 
> I also think that creating a new language from which we generate
> tracepoints for both is an overkill though. It seems that we don't have
> a perfect way to support both lttng and Perfetto...
> 
> BTW, I tried lex and yacc to parse lttng macros, and tbh it's very painful
> that it's very hard to use lex to recognize C/C++ type as a token... So
> yeah if we end up deciding to take this route, a homemade parser from
> scratch makes more sense to me.
> 
> So let me list the approaches with pros and cons:
> 1. Generate Perfetto code from lttng macros.
>   Pros: Easy to update libcamera tracepoint macros.
>   Cons: Add maintenance work with the additional parser and code 
> generator; hard to implement Perfetto features that lttng doesn't have.

This makes me wonder, if Perfetto is a superset of the features of
lttng, why not go the other way; use Perfetto tracepoints as the main
authority and then generate lttng tracepoints from those.

What are your thoughts?

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

If Perfetto gets added to the package repositories in linux
distributions then I think it would be used :)

Not being able to sudo apt install perfetto is a pretty big blocker.


Thanks,


Paul

> 
> Chinglin, please help amend anything I missed as well :)
> 
> BR,
> Harvey
> 
> 
>     [2] utils/ipc/meson.build
>     [3] include/libcamera/ipa/meson.build
>     [4] src/libcamera/ipa/meson.build
> 
>     Would you be able to do this?
> 
> 
>     Paul
> 
>     > diff --git a/src/libcamera/tracepoints.cpp b/src/libcamera/
>     tracepoints.cpp
>     > index 0173b75a..a07ea531 100644
>     > --- a/src/libcamera/tracepoints.cpp
>     > +++ b/src/libcamera/tracepoints.cpp
>     > @@ -4,7 +4,18 @@
>     >   *
>     >   * tracepoints.cpp - Tracepoints with lttng
>     >   */
>     > +
>     > +#if HAVE_PERFETTO
>     > +
>     > +#include "libcamera/internal/tracepoints.h"
>     > +
>     > +PERFETTO_TRACK_EVENT_STATIC_STORAGE();
>     > +
>     > +#else
>     > +
>     >  #define TRACEPOINT_CREATE_PROBES
>     >  #define TRACEPOINT_DEFINE
>     > 
>     >  #include "libcamera/internal/tracepoints.h"
>     > +
>     > +#endif /* HAVE_PERFETTO */
>     > --
>     > 2.39.0.314.g84b9a713c41-goog
>     >
>

Hi Paul,

On Tue, Sep 10, 2024 at 12:30 AM Paul Elder <paul.elder@ideasonboard.com>
wrote:

> Hi Harvey,
>
> <snip>
>
> >     So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments
> >     and fields and how to obtain the fields from the args. We can
> >     instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only
> >     have to specify the args, and the fields are already defined and come
> >     from the class. This reduces duplication in the tracepoint
> definitions.
> >
> >     I'm wondering from what you've implemented here that perfetto doesn't
> >     support this? I don't see anything in the perfetto docs that suggest
> >     that this is possible either. Although we can mitigate this issue by
> >     generating the perfetto tracepoints :) (this is another reason why I
> >     think we should use the lttng tracepoints definition as the main one
> and
> >     generate the perfetto ones from those)
> >
> >
> >
> > I've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that
> > there are limited features in lttng [1], and if we use the lttng
> tracepoints
> > definition as the source of truth, it's hard/weird to implement some
> Perfetto
> > concepts, like nested slices (lifetime of trace events) and flows [2].
>
> Ah ok, I see the problem.
>
> >
> > It's not impossible, as you mentioned above, that we could detect the
> prefix
> > and suffix of each libcamera/lttng macro. However, it's a weird code
> parser
> > and generator design.
> >
> > [1]: https://lttng.org/man/3/lttng-ust/v2.8/
> > [2]: https://perfetto.dev/docs/instrumentation/track-events
> >
> >
>
> <snip>
>
> >
> >
> > I agree that in the current functionalities, it's not that hard to
> implement
> > a homemade C/python parser, and generate Perfetto code with jinja2.
> > However, we not only "limit" the functionalities of Perfetto with lttng's
> > definition, but also add another layer to be maintained.
> > It's true that having duplication of libcamera tracepoint definitions
> > requires more maintenance work when updating the libcamera
> > tracepoints, while the effort to maintain the parser and code generation
> > is also non-negligible.
> >
> > I also think that creating a new language from which we generate
> > tracepoints for both is an overkill though. It seems that we don't have
> > a perfect way to support both lttng and Perfetto...
> >
> > BTW, I tried lex and yacc to parse lttng macros, and tbh it's very
> painful
> > that it's very hard to use lex to recognize C/C++ type as a token... So
> > yeah if we end up deciding to take this route, a homemade parser from
> > scratch makes more sense to me.
> >
> > So let me list the approaches with pros and cons:
> > 1. Generate Perfetto code from lttng macros.
> >   Pros: Easy to update libcamera tracepoint macros.
> >   Cons: Add maintenance work with the additional parser and code
> > generator; hard to implement Perfetto features that lttng doesn't have.
>
> This makes me wonder, if Perfetto is a superset of the features of
> lttng, why not go the other way; use Perfetto tracepoints as the main
> authority and then generate lttng tracepoints from those.
>
> What are your thoughts?
>

I don't think Perfetto MACROs is a superset of lttng MACROs. If we go
with this approach, there are some features dropped (like integer_hex
and enum, and the concept of TP_ARGS), and the code would be messy.
Take request_complete_buffer as the example:

```
# Perfetto
TRACE_EVENT("libcamera", "request_complete_buffer",
      "request_private_ptr", reinterpret_cast<uintptr_t>(req),
      "cookie", req->_o<libcamera::Request>()->cookie(),
      "status", req->_o<libcamera::Request>()->status(),
      "buffer_ptr", reinterpret_cast<uintptr_t>(buf),
      "buffer_status", buf->metadata().status);
```

It'd be how perfetto implements the macro. Originally lttng has only two
inputs: `libcamera::Request::internal *req` and
`libcamera::FrameBuffer *buf`. Unless we're going to do complicated
type diagnosis and simplification, we might end up with having five
inputs as well. Also, I don't know how to indicate the usage of
ctf_integer_hex (for `req` and `buf`) and ctf_enum (for status).

I'd still prefer the 3rd or 4th approach.


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

BR,
Harvey


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

Hi Harvey,

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

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

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

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

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

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

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

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

Hi libcamera developers,

I've consulted with Google Perfetto team, and yes, we agree
that it makes more sense to have one debugging tool as the
source of truth and generate code for the other tool, to ease
the development effort in the future.

Please also share your concerns on the two approaches:

On Sat, Sep 14, 2024 at 1:14 AM Lalit Maganti <lalitm@google.com> wrote:

>
>
> On Fri, 13 Sept 2024 at 18:08, Cheng-Hao Yang <chenghaoyang@google.com>
> wrote:
>
>> Thank you Lalit for the quick and thorough response,
>>
>> Do you mind if I directly forward your messages to libcamera upstream
>> directly?
>> I'll wait for a couple of days until everyone has the chance to speak
>> out, and do a brief summary though.
>>
>
> Sure that sounds good.
>
>
> Responses inline:
>>
>> On Fri, Sep 13, 2024 at 6:19 PM Lalit Maganti <lalitm@google.com> wrote:
>>
>>> Also +Daniele Di Proietto <ddiproietto@google.com> is the C SDK better
>>> at being closer to LTTng in ethos? I know it allows e.g. defining custom
>>> protos directly in code and writing out those? Would that be a better fit
>>> for what these folks are trying to do?
>>>
>>
>> Yeah I forgot to mention that at least in CrOS, we're using cros-built
>> Perfetto C SDK in libcamera.
>>
>>
>>>
>>> On Fri, 13 Sept 2024, 10:54 Lalit Maganti, <lalitm@google.com> wrote:
>>>
>>>> Please do feel free to setup some time with us (you can book slots at
>>>> go/perfetto-office-hours
>>>> <https://goto.google.com/perfetto-office-hours>) or reach out by IM
>>>> btw to speed iteration on this up if you have more questions/want to
>>>> discuss more.
>>>>
>>>> On Fri, 13 Sept 2024 at 10:53, Lalit Maganti <lalitm@google.com> wrote:
>>>>
>>>>> Hi Harvey,
>>>>>
>>>>> Responses inline.
>>>>>
>>>>> On Fri, 13 Sept 2024 at 08:56, Cheng-Hao Yang <chenghaoyang@google.com>
>>>>> wrote:
>>>>>
>>>>>>  Hi Primiano and Perfetto developers,
>>>>>>
>>>>>> This is Harvey, working in CrOS Camera team on libcamera project
>>>>>> <https://libcamera.org/>. I need some help from you, as I'm working
>>>>>> on supporting Perfetto in libcamera upstream, which currently uses lttng as
>>>>>> the tracing tool. Libcamera upstream has the concern that Perfetto is not
>>>>>> added in the package repositories in linux distributions, and the way to
>>>>>> build it in linux doesn't match the philosophy of Linux distributions.
>>>>>>
>>>>>> We're considering supporting both tracing tools in libcamera, while
>>>>>> libcamera upstream strongly prefers to have only one source of truth, which
>>>>>> means to maintain only one version of trace macros/configs, and generate
>>>>>> macros for the other (or both of them). This email thread contains the
>>>>>> patch <https://patchwork.libcamera.org/patch/18040/> of 3rd approach
>>>>>> listed below.
>>>>>>
>>>>>> I need your help on mainly two things:
>>>>>> 1. An official reply to confirm that it is or isn't a priority to add
>>>>>> Perfetto in the package repositories in linux distributions (I assume it
>>>>>> isn't though). After providing this info, I can confirm with libcamera
>>>>>> upstream our consensus of Perfetto support and responsibilities in the
>>>>>> future.
>>>>>>
>>>>>
>>>>> It is not a priority (and historically not been a priority) *but* at
>>>>> least for Debian we already did a bunch of the work to make a package for
>>>>> distributing Perfetto (see here
>>>>> <https://source.corp.google.com/h/android/platform/superproject/main/+/main:external/perfetto/debian/;l=1?q=f:perfetto%20f:debian&sq=repo:android%2Fplatform%2Fsuperproject%2Fmain%20b:main>).
>>>>> If someone from your side was willing to do the work to submit this
>>>>> package, we'd be happy to keep it updated and maintained going forward.
>>>>>
>>>>> So for us:
>>>>>
>>>>>    1. Is someone from your side willing to take the work we've done
>>>>>    and push it to completion?
>>>>>    2. Are there any other distros apart from Debian which libcamera
>>>>>    would need us to be in?
>>>>>
>>>>> 1. Unfortunately, I don't think we have the bandwidth to help push it
>> to completion in the current stage...
>> 2. I believe there are at least Fedora and Raspberry. I'll let the
>> libcamera upstream developers correct me.
>>
>
> That's fair. I think in that case, it would be difficult for us to get it
> supported to the level that they want as well unfortunately. We're already
> resource constrained as it is and with Android and AL work, I don't think
> we'll have the bandwidth to deal with all the work for upstreaming into
> debian and all the other distros.
>
>
>>>>>
>>>>>> 2. Take a brief look at the approaches below, and check if I missed
>>>>>> something. You may focus on the 1st or 5th, which I'm currently inclined
>>>>>> with.
>>>>>>
>>>>>
>>>>>> Here's the list of approaches that we can think of:
>>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> An option but as you say, it does not support a bunch of features in
>>>>> Perfetto which LTTNG does not have.
>>>>>
>>>>
This one, which you might prefer.


>
>>>>>
>>>>>>
>>>>>> 2. Create a new language/config 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.
>>>>>>
>>>>>
>>>>> Yeah would not go here.
>>>>>
>>>>> 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.
>>>>>>
>>>>>
>>>>> Inevitably things would get out of sync, I can see why they are
>>>>> against this.
>>>>>
>>>>> 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.
>>>>>>
>>>>>
>>>>> As above, same deal.
>>>>>
>>>>>
>>>>>> 5. Add libcamera macro usages, and generate lttng macros with codegen.
>>>>>>   Pros: It supports all perfetto features.
>>>>>>   Cons: Some lttng-only features (like ctf_integer_hex and ctf_enum)
>>>>>> are not available without weird hacks.
>>>>>>
>>>>>
>>>>> If there are things like this, I think we'd be happy to work with you
>>>>> on getting those gaps resolved (if it's feasible, I haven't looked into the
>>>>> details in this particular case, it might not be).
>>>>>
>>>>
>> Yeah it might be the best case for us now.
>>
>
> Cool sounds good :)
>

Or this one, that Google prefers. We might need to find
a way to support hex & enum in this case though.


>
> --------------
>>>>>>
>>>>>> As libcamera upstream has a strong opinion, I might end up going with
>>>>>> the 1st or 5th approach. The reason to consider the 1st is that currently
>>>>>> the libcamera trace points might be better off with Perfetto counters (or
>>>>>> slices with minimum lifetime), instead of slices. Please correct me if I'm
>>>>>> wrong, but I think Perfetto slices are most useful for nested function
>>>>>> calls. However, a lot of libcamera operations are triggered by callbacks
>>>>>> (in libcamera, Signal).
>>>>>> I tried Perfetto flow yesterday, and it works on libcamera. We can
>>>>>> possibly add a hack in the potential codegen to set it up though.
>>>>>>
>>>>>
>>>>> Perfetto slices are not just for sync function calls: you can create
>>>>> synthetic sync stacks from them based on callback lifecycles: in
>>>>> Chrome/ChromeOS where everything is based on TaskRunner (thread loops) and
>>>>> callbacks, Perfetto is used very extensively and along with flows, allows
>>>>> visualizing async stuff just fine.
>>>>>
>>>>> Counters are not a good fit for visualizing latencies of things which
>>>>> given the nature of libcamera is almost certainly what you want.
>>>>>
>>>>
>> Thanks for the info! Flow is very useful indeed. I'll do some more
>> studies on the synthetic sync stacks, and check the best way to add traces
>> in libcamera. It might be very different from the current lttng ones
>> though, I believe.
>>
>
> I mean an option here is to do LTTNG -> Perfetto conversion and then add
> some custom instrumentation in the places where it really helps having
> custom Perfetto instrumentation maybe? Maybe the libcamera folk would be
> more amenable to that?
>
>
>>
>>>>> Let me know if there's anything I haven't clarified yet.
>>>>>>
>>>>>> Many thanks!
>>>>>> Harvey
>>>>>>
>>>>>> ---------- Forwarded message ---------
>>>>>> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>> Date: Wed, Sep 11, 2024 at 7:10 PM
>>>>>> Subject: Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with
>>>>>> perfetto in ChromeOS
>>>>>> To: Cheng-Hao Yang <chenghaoyang@chromium.org>
>>>>>> Cc: Paul Elder <paul.elder@ideasonboard.com>, Chinglin Yu <
>>>>>> chinglinyu@chromium.org>, <libcamera-devel@lists.libcamera.org>,
>>>>>> Harvey Yang <chenghaoyang@google.com>
>>>>>>
>>>>>>
>>>>>> Hi Harvey,
>>>>>>
>>>>>> On Wed, Sep 11, 2024 at 06:05:18PM +0800, Cheng-Hao Yang wrote:
>>>>>> > On Tue, Sep 10, 2024 at 12:30 AM Paul Elder wrote:
>>>>>> > > Hi Harvey,
>>>>>> > >
>>>>>> > > <snip>
>>>>>> > >
>>>>>> > > >     So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines
>>>>>> arguments
>>>>>> > > >     and fields and how to obtain the fields from the args. We
>>>>>> can
>>>>>> > > >     instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE,
>>>>>> where we only
>>>>>> > > >     have to specify the args, and the fields are already
>>>>>> defined and come
>>>>>> > > >     from the class. This reduces duplication in the tracepoint
>>>>>> definitions.
>>>>>> > > >
>>>>>> > > >     I'm wondering from what you've implemented here that
>>>>>> perfetto doesn't
>>>>>> > > >     support this? I don't see anything in the perfetto docs
>>>>>> that suggest
>>>>>> > > >     that this is possible either. Although we can mitigate this
>>>>>> issue by
>>>>>> > > >     generating the perfetto tracepoints :) (this is another
>>>>>> reason why I
>>>>>> > > >     think we should use the lttng tracepoints definition as the
>>>>>> main one and
>>>>>> > > >     generate the perfetto ones from those)
>>>>>>
>>>>>
>>>>> The LTTNG model is very very different from the Perfetto model. For
>>>>> us, the equivalent of LTTNG "tracepoints" are defined directly in Perfetto
>>>>> source code. You wouldn't see it on the libcamera side as you use the
>>>>> macros to directly write these (they're really protos) without having a
>>>>> "definition" in the libcamera source code.
>>>>>
>>>>>
>>>>>> > > >
>>>>>> > > > I've discussed with ChromeOS Perfetto TL chinglinyu@, and we
>>>>>> found that
>>>>>> > > > there are limited features in lttng [1], and if we use the
>>>>>> lttng tracepoints
>>>>>> > > > definition as the source of truth, it's hard/weird to implement
>>>>>> some Perfetto
>>>>>> > > > concepts, like nested slices (lifetime of trace events) and
>>>>>> flows [2].
>>>>>> > >
>>>>>> > > Ah ok, I see the problem.
>>>>>> > >
>>>>>> > > > It's not impossible, as you mentioned above, that we could
>>>>>> detect the prefix
>>>>>> > > > and suffix of each libcamera/lttng macro. However, it's a weird
>>>>>> code parser
>>>>>> > > > and generator design.
>>>>>> > > >
>>>>>> > > > [1]: https://lttng.org/man/3/lttng-ust/v2.8/
>>>>>> > > > [2]: https://perfetto.dev/docs/instrumentation/track-events
>>>>>> > >
>>>>>> > > <snip>
>>>>>> > >
>>>>>> > > > I agree that in the current functionalities, it's not that hard
>>>>>> to implement
>>>>>> > > > a homemade C/python parser, and generate Perfetto code with
>>>>>> jinja2.
>>>>>> > > > However, we not only "limit" the functionalities of Perfetto
>>>>>> with lttng's
>>>>>> > > > definition, but also add another layer to be maintained.
>>>>>> > > > It's true that having duplication of libcamera tracepoint
>>>>>> definitions
>>>>>> > > > requires more maintenance work when updating the libcamera
>>>>>> > > > tracepoints, while the effort to maintain the parser and code
>>>>>> generation
>>>>>> > > > is also non-negligible.
>>>>>> > > >
>>>>>> > > > I also think that creating a new language from which we generate
>>>>>> > > > tracepoints for both is an overkill though. It seems that we
>>>>>> don't have
>>>>>> > > > a perfect way to support both lttng and Perfetto...
>>>>>> > > >
>>>>>> > > > BTW, I tried lex and yacc to parse lttng macros, and tbh it's
>>>>>> very painful
>>>>>> > > > that it's very hard to use lex to recognize C/C++ type as a
>>>>>> token... So
>>>>>> > > > yeah if we end up deciding to take this route, a homemade
>>>>>> parser from
>>>>>> > > > scratch makes more sense to me.
>>>>>> > > >
>>>>>> > > > So let me list the approaches with pros and cons:
>>>>>> > > > 1. Generate Perfetto code from lttng macros.
>>>>>> > > >   Pros: Easy to update libcamera tracepoint macros.
>>>>>> > > >   Cons: Add maintenance work with the additional parser and code
>>>>>> > > > generator; hard to implement Perfetto features that lttng
>>>>>> doesn't have.
>>>>>> > >
>>>>>> > > This makes me wonder, if Perfetto is a superset of the features of
>>>>>> > > lttng, why not go the other way; use Perfetto tracepoints as the
>>>>>> main
>>>>>> > > authority and then generate lttng tracepoints from those.
>>>>>> > >
>>>>>> > > What are your thoughts?
>>>>>> >
>>>>>> > I don't think Perfetto MACROs is a superset of lttng MACROs. If we
>>>>>> go
>>>>>> > with this approach, there are some features dropped (like
>>>>>> integer_hex
>>>>>> > and enum, and the concept of TP_ARGS), and the code would be messy.
>>>>>> > Take request_complete_buffer as the example:
>>>>>> >
>>>>>> > ```
>>>>>> > # Perfetto
>>>>>> > TRACE_EVENT("libcamera", "request_complete_buffer",
>>>>>> >       "request_private_ptr", reinterpret_cast<uintptr_t>(req),
>>>>>> >       "cookie", req->_o<libcamera::Request>()->cookie(),
>>>>>> >       "status", req->_o<libcamera::Request>()->status(),
>>>>>> >       "buffer_ptr", reinterpret_cast<uintptr_t>(buf),
>>>>>> >       "buffer_status", buf->metadata().status);
>>>>>> > ```
>>>>>> >
>>>>>> > It'd be how perfetto implements the macro. Originally lttng has
>>>>>> only two
>>>>>> > inputs: `libcamera::Request::internal *req` and
>>>>>> > `libcamera::FrameBuffer *buf`. Unless we're going to do complicated
>>>>>> > type diagnosis and simplification, we might end up with having five
>>>>>> > inputs as well.
>>>>>>
>>>>>
I take this back. As we'll most likely have a self-defined
function that takes the two inputs with the correct C/C++
types, we might still be able to maintain the input size for
lttng macros.

BR,
Harvey


> Also, I don't know how to indicate the usage of
>>>>>> > ctf_integer_hex (for `req` and `buf`) and ctf_enum (for status).
>>>>>> >
>>>>>> > I'd still prefer the 3rd or 4th approach.
>>>>>>
>>>>>> The 4th option, duplicating trace points in the libcamera sources, is
>>>>>> the one I like the least. The 3rd option is also something I don't
>>>>>> like
>>>>>> much. It means that anyone adding trace points will need to do extra
>>>>>> work to support two trace systems. Testing it locally will be hard,
>>>>>> even
>>>>>> for build testing. We could possibly handle the build testing with CI
>>>>>> (see below), even if that will be way less convenient that doing local
>>>>>> builds. Runtime testing is likely out of question.
>>>>>>
>>>>>> I'm not comfortable with the amount of extra work this will cause to
>>>>>> everybody to support a Google-only trace system. I'm willing to find a
>>>>>> compromise though, if Google could do the necessary work needed to
>>>>>> package Perfetto in distributions, I could accept extra work on our
>>>>>> side
>>>>>> too.
>>>>>>
>>>>>> > > > 2. Creating a new language from which we generate both Perfetto
>>>>>> > > > and lttng macros.
>>>>>> > > >   Pros: Easy to update libcamera tracepoint macros.
>>>>>> > > >   Cons: Probably an overkill; hard to cover all features from
>>>>>> both
>>>>>> > > > Perfetto and lttng.
>>>>>> > > >
>>>>>> > > > 3. As this patch implements: have duplicated libcamera
>>>>>> tracepoint
>>>>>> > > > definitions.
>>>>>> > > >   Pros: Easy to add libcamera tracepoint macros to cover all
>>>>>> features
>>>>>> > > > from Perfetto & lttng.
>>>>>> > > >   Cons: Developer needs to update both sides when the
>>>>>> definition of
>>>>>> > > > libcamera tracepoints changes; Unused macro definitions might be
>>>>>> > > > adopted in both sides, if a feature is only available in one
>>>>>> trace
>>>>>> > > library.
>>>>>> > > >
>>>>>> > > > 4. Add another set of libcamera tracepoints for Perfetto, or
>>>>>> just use
>>>>>> > > > Perfetto macros directly in libcamera's source code.
>>>>>> > > >   Pros: Two isolated trace stacks that don't influence each
>>>>>> other. All
>>>>>> > > > features can be implemented easily.
>>>>>> > > >   Cons: Duplicated trace code in source code that might lead to
>>>>>> > > > confusion; developers also need to update both macros when a
>>>>>> > > > new trace is needed.
>>>>>> > > >
>>>>>> > > > I think approach 3rd and 4th are easier to maintain in the long
>>>>>> > > > term. WDYT?
>>>>>> > > >
>>>>>> > > > Also, I'd like to know if you think Perfetto will be used in
>>>>>> other linux
>>>>>> > > > distributions than ChromeOS or Android.
>>>>>> > >
>>>>>> > > If Perfetto gets added to the package repositories in linux
>>>>>> > > distributions then I think it would be used :)
>>>>>> > >
>>>>>> > > Not being able to sudo apt install perfetto is a pretty big
>>>>>> blocker.
>>>>>> >
>>>>>> > Yeah I understand, while Perfetto is self-contained and simple to
>>>>>> build
>>>>>> > in linux distros.
>>>>>>
>>>>>> Do you mean by running the tools/install-build-deps script that
>>>>>> downloads toolchain binaries from google servers ? That doesn't really
>>>>>> match the philosophy of Linux distributions.
>>>>>>
>>>>>
>>>>> Yeah not accepting this makes sense I'm sympathetic to this.
>>>>>
>>>>>
>>>>>>
>>>>>> > May we consider testing it on the gitlab CI, after we
>>>>>> > somehow add the support?
>>>>>>
>>>>>> If we add support for it I certainly would like to see it being tested
>>>>>> in CI, yes. After a quick look at the Perfetto build system, it looks
>>>>>> like adding Perfetto to the CI docker containers may not be trivial.
>>>>>> Someone will need to volunteer.
>>>>>>
>>>>>> > > > Chinglin, please help amend anything I missed as well :)
>>>>>>
>>>>>> --
>>>>>> Regards,
>>>>>>
>>>>>> Laurent Pinchart
>>>>>>
>>>>>>
>>>>>> --
>>>>>> BR,
>>>>>> Harvey Yang
>>>>>>
>>>>>> --
>>>>>> You received this message because you are subscribed to the Google
>>>>>> Groups "Perfetto-dev" group.
>>>>>> To unsubscribe from this group and stop receiving emails from it,
>>>>>> send an email to perfetto-dev+unsubscribe@google.com.
>>>>>> To view this discussion on the web visit
>>>>>> https://groups.google.com/a/google.com/d/msgid/perfetto-dev/CAC%3DwSGWxc4W9nZOoieTMhG9h-Kh%2BwH%3D%3DiPfNwxqd0ybNntVFWQ%40mail.gmail.com
>>>>>> <https://groups.google.com/a/google.com/d/msgid/perfetto-dev/CAC%3DwSGWxc4W9nZOoieTMhG9h-Kh%2BwH%3D%3DiPfNwxqd0ybNntVFWQ%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>>>>> .
>>>>>>
>>>>>
>>
>> --
>> BR,
>> Harvey Yang
>>
>