[{"id":26165,"web_url":"https://patchwork.libcamera.org/comment/26165/","msgid":"","date":"2022-12-27T04:08:57","subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Harvey,\n\nOn Tue, Dec 20, 2022 at 07:05:23AM +0000, Harvey Yang via libcamera-devel wrote:\n> As ChromeOS is using perfetto (project named as CrOSetto). When ChromeOS\n> uses libcamera, it should use perfetto to collect traces instead of\n> lttng.\n\nI'll start with the elephant in the room: we cannot have duplicate\ntracepoint definitions. There must be only one definition of the\ntracepoints, and either one or both of perfetto's and lttng's\ntracepoints must be generated from that.\n\nFor instance, I've already got a series pending [1] that expands on the\nrequest tracepoints quite a bit; it's clearly unscalable to deal with\ntwo duplicate sets of tracepoints (I'm not sure why I have to defend\ndeduplication, but anyway just trying to be clear about my stance).\n\n[1] https://patchwork.libcamera.org/project/libcamera/list/?series=3668\n\nMore on this in the relevant section in the patch.\n\n> \n> Signed-off-by: Harvey Yang \n> ---\n> Documentation/guides/tracing.rst | 125 ++++++++++++++----\n> include/libcamera/internal/tracepoints.h.in | 38 +++++-\n> .../internal/tracepoints/meson.build | 25 ++--\n> .../internal/tracepoints/pipeline.perfetto | 10 ++\n> .../internal/tracepoints/request.perfetto | 30 +++++\n> meson.build | 7 +-\n> src/android/cros/camera3_hal.cpp | 5 +\n> src/android/cros/meson.build | 1 +\n> src/libcamera/meson.build | 14 +-\n> src/libcamera/perfetto/meson.build | 6 +\n> src/libcamera/perfetto/pipeline_perfetto.cpp | 24 ++++\n> src/libcamera/perfetto/request_perfetto.cpp | 73 ++++++++++\n> src/libcamera/tracepoints.cpp | 11 ++\n> 13 files changed, 325 insertions(+), 44 deletions(-)\n> create mode 100644 include/libcamera/internal/tracepoints/pipeline.perfetto\n> create mode 100644 include/libcamera/internal/tracepoints/request.perfetto\n> create mode 100644 src/libcamera/perfetto/meson.build\n> create mode 100644 src/libcamera/perfetto/pipeline_perfetto.cpp\n> create mode 100644 src/libcamera/perfetto/request_perfetto.cpp\n> \n> diff --git a/Documentation/guides/tracing.rst b/Documentation/guides/tracing.rst\n> index ae960d85..bd5b3453 100644\n> --- a/Documentation/guides/tracing.rst\n> +++ b/Documentation/guides/tracing.rst\n> @@ -16,33 +16,50 @@ at periodic points in time. This can be done with other tools such as\n> callgrind, perf, gprof, etc., without modification to the application,\n> and is out of scope for this guide.\n> \n> +Library: Perfetto vs lttng-ust\n> +------------------------------\n> +\n> +To integrate with CrOS tracing infrastructure, which uses perfetto (or called\n\nThe first instance of \"CrOS\" should probably be spelled out.\n\n> +CrOSetto in CrOS), we implement the tracepoint macros (will be described below)\n> +with two different libraries: CrOS with perfetto, and the rest with lttng-ust.\n> +\n> Compiling\n> ---------\n> \n> -To compile libcamera with tracing support, it must be enabled through the\n> -meson ``tracing`` option. It depends on the lttng-ust library (available in the\n> -``liblttng-ust-dev`` package for Debian-based distributions).\n> -By default the tracing option in meson is set to ``auto``, so if\n> -liblttng is detected, it will be enabled by default. Conversely, if the option\n> -is set to disabled, then libcamera will be compiled without tracing support.\n> +To compile libcamera with tracing support in CrOS, it must be enabled through\n\ns/ in CrOS//\n\n> +the meson ``tracing`` option. In CrOS, it depends on the perfetto library,\n> +which is by default available in CrOS; In the rest, it depends on the lttng-ust\n\ns/;/./\n\ns/the rest/non-CrOS/\n\n> +library (available in the ``liblttng-ust-dev`` package for Debian-based\n> +distributions).\n> +By default the tracing option in meson is set to ``auto``, so if the library is\n> +detected, it will be enabled by default. Conversely, if the option is set to\n> +disabled, then libcamera will be compiled without tracing support.\n> \n> Defining tracepoints\n> --------------------\n> \n> libcamera already contains a set of tracepoints. To define additional\n> -tracepoints, create a file\n> -``include/libcamera/internal/tracepoints/{file}.tp``, where ``file`` is a\n> -reasonable name related to the category of tracepoints that you wish to\n> -define. For example, the tracepoints file for the Request object is called\n> -``request.tp``. An entry for this file must be added in\n> -``include/libcamera/internal/tracepoints/meson.build``.\n> -\n> -In this tracepoints file, define your tracepoints `as mandated by lttng\n> +tracepoints, create two files ``{file}.tp`` and ``{file}.perfetto``, for lttng\n> +and perfetto respectively, under ``include/libcamera/internal/tracepoints/``,\n> +and the perfetto implementation file ``src/libcamera/{file}_perfetto.cpp``,\n> +where ``file`` is a reasonable name related to the category of tracepoints that\n> +you wish to define. For example, the tracepoints files for the Request object is\n> +called ``request.tp`` and ``request.perfetto``. Entries for the files must be\n> +added in ``include/libcamera/internal/tracepoints/meson.build``.\n> +\n> +In the perfetto tracepoints files, declare the tracepoint functions in\n> +``{file}.perfetto``, and define them in ``{file}_perfetto.cpp``, `as mandated\n> +by perfetto `_.\n> +Currently the only enabled perfetto::Category is ``libcamera``. If you intend to\n> +use another category, remember to define it in\n> +``include/libcamera/internal/tracepoints.h.in``, and enable it when collecting\n> +traces.\n\nThese sections will have to be fixed.\n\n> +\n> +In the lttng tracepoints file, define your tracepoints `as mandated by lttng\n> `_. The header boilerplate must *not* be\n> included (as it will conflict with the rest of our infrastructure), and\n> only the tracepoint definitions (with the ``TRACEPOINT_*`` macros) should be\n> included.\n> -\n> All tracepoint providers shall be ``libcamera``. According to lttng, the\n> tracepoint provider should be per-project; this is the rationale for this\n> decision. To group tracepoint events, we recommend using\n> @@ -68,9 +85,9 @@ Then to use the tracepoint:\n> \n> ``LIBCAMERA_TRACEPOINT({tracepoint_event}, args...)``\n> \n> -This macro must be used, as opposed to lttng's macros directly, because\n> -lttng is an optional dependency of libcamera, so the code must compile and run\n> -even when lttng is not present or when tracing is disabled.\n> +This macro must be used, as opposed to perfetto's or lttng's macros directly,\n> +because tracing support is optional of libcamera, so the code must compile and\n\ns/of/in/\n\n> +run even when perfetto or lttng are not present or when tracing is disabled.\n> \n> The tracepoint provider name, as declared in the tracepoint definition, is not\n> included in the parameters of the tracepoint.\n> @@ -87,21 +104,75 @@ respectively. These are the tracepoints that our sample analysis script\n> (see \"Analyzing a trace\") scans for when computing statistics on IPA call time.\n> \n> Using tracepoints (from an application)\n> ----------------------------------------\n> +---------------------------------------------\n> \n> As applications are not part of libcamera, but rather users of libcamera,\n> applications should seek their own tracing mechanisms. For ease of tracing\n> the application alongside tracing libcamera, it is recommended to also\n> -`use lttng `_.\n> +`use lttng `_,\n> +or `use perfetto `_.\n> \n> Using tracepoints (from closed-source IPA)\n> -------------------------------------------\n> +------------------------------------------------\n\nUnderline length.\n\n> +\n> +Similar to applications, closed-source IPAs can simply use perfetto or lttng\n> +on their own, or any other tracing mechanism if desired.\n> +\n> +Collecting a perfetto trace\n> +---------------------------\n> +\n> +A trace can be collected with the following steps:\n> +\n> +1. Start `traced` if it hasn't been started.\n> +.. code-block:: bash\n> +\n> + start traced\n> +\n> +2. Start a consumer that includes “track_event” data source in the trace\n> + config, and “libcamera” category.\n> +.. code-block:: bash\n> +\n> + perfetto -c - --txt -o /tmp/perfetto-trace \\\n\n(Reply to your comment in the other email) I think this is sufficient,\nas apparently the option for a config file is documented in the help\ntext.\n\n> + < +\n> + buffers: {\n> + size_kb: 63488\n> + fill_policy: DISCARD\n> + }\n> + buffers: {\n> + size_kb: 2048\n> + fill_policy: DISCARD\n> + }\n> + data_sources: {\n> + config {\n> + name: \"track_event\"\n> + track_event_config {\n> + enabled_categories: \"libcamera\"\n> + }\n> + }\n> + }\n> + duration_ms: 10000\n> +\n> + EOF\n> +\n> +3. Execute the libcamera behavior you intend to trace during the set duration\n> + (10000 ms) in the above example.\n> +\n> +4. After the consumer (cmd `perfetto`) is done, you can find the trace result\n> + in ``/tmp/perfetto-trace``.\n> +\n> +Analyzing a perfetto trace\n> +--------------------------\n> +\n> +Follow the `guide `_ to\n> +visualize the trace.\n> \n> -Similar to applications, closed-source IPAs can simply use lttng on their own,\n> -or any other tracing mechanism if desired.\n> +In other words, upload the trace to `Perfetto UI `_,\n> +where you can check the timeline of tracepoints on each process/thread. You can\n> +also run SQL queries to do analysis.\n> \n> -Collecting a trace\n> -------------------\n> +Collecting an lttng trace\n> +------------------------\n\nUnderline length.\n\n> \n> A trace can be collected fairly simply from lttng:\n> \n> @@ -123,8 +194,8 @@ viewed by: ``lttng view -t $PATH_TO_TRACE``, where ``$PATH_TO_TRACE`` is the\n> path that was printed when the session was created. This is the same path that\n> is used when analyzing traces programatically, as described in the next section.\n> \n> -Analyzing a trace\n> ------------------\n> +Analyzing a lttng trace\n\ns/a/an/\n\n> +-----------------------\n> \n> As mentioned above, while an lttng tracing session exists and the trace is not\n> running, the trace output can be viewed as text by ``lttng view``.\n> diff --git a/include/libcamera/internal/tracepoints.h.in b/include/libcamera/internal/tracepoints.h.in\n> index d0fc1365..b6c4abea 100644\n> --- a/include/libcamera/internal/tracepoints.h.in\n> +++ b/include/libcamera/internal/tracepoints.h.in\n> @@ -9,7 +9,24 @@\n> #ifndef __LIBCAMERA_INTERNAL_TRACEPOINTS_H__\n> #define __LIBCAMERA_INTERNAL_TRACEPOINTS_H__\n> \n> -#if HAVE_TRACING\n> +#if HAVE_PERFETTO\n> +\n> +#include \n> +\n> +PERFETTO_DEFINE_CATEGORIES(\n> +\tperfetto::Category(\"libcamera\")\n> +\t\t.SetDescription(\"Events from libcamera\"));\n> +\n> +#define LIBCAMERA_TRACEPOINT(t_name, ...) \\\n> +LIBCAMERA_TRACE_EVENT_##t_name(__VA_ARGS__)\n> +\n> +#define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \\\n> +LIBCAMERA_TRACE_EVENT_ipa_call_begin(#pipe, #func)\n> +\n> +#define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \\\n> +LIBCAMERA_TRACE_EVENT_ipa_call_end(#pipe, #func)\n> +\n> +#elif HAVE_LTTNG /* !HAVE_PERFETTO */\n> #define LIBCAMERA_TRACEPOINT(...) tracepoint(libcamera, __VA_ARGS__)\n> \n> #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \\\n> @@ -18,7 +35,7 @@ tracepoint(libcamera, ipa_call_begin, #pipe, #func)\n> #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \\\n> tracepoint(libcamera, ipa_call_end, #pipe, #func)\n> \n> -#else\n> +#else /* !HAVE_PERFETTO && !HAVE_LTTNG */\n> \n> namespace {\n> \n> @@ -34,12 +51,17 @@ inline void unused([[maybe_unused]] Args&& ...args)\n> #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func)\n> #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func)\n> \n> -#endif /* HAVE_TRACING */\n> +#endif /* HAVE_PERFETTO */\n> \n> #endif /* __LIBCAMERA_INTERNAL_TRACEPOINTS_H__ */\n> \n> +#if HAVE_PERFETTO || HAVE_LTTNG\n> +\n> +#if HAVE_PERFETTO\n> \n> -#if HAVE_TRACING\n> +#include \n> +\n> +#else /* HAVE_LTTNG */\n> \n> #undef TRACEPOINT_PROVIDER\n> #define TRACEPOINT_PROVIDER libcamera\n> @@ -47,15 +69,21 @@ inline void unused([[maybe_unused]] Args&& ...args)\n> #undef TRACEPOINT_INCLUDE\n> #define TRACEPOINT_INCLUDE \"{{path}}\"\n> \n> +#endif /* HAVE_PERFETTO */\n> +\n> #if !defined(INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H) || defined(TRACEPOINT_HEADER_MULTI_READ)\n> #define INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H\n\nI'm pretty sure #endif /* HAVE_PERFETTO */ should come here (unless\nperfetto also needs this? Does it?).\n\n> \n> +#if HAVE_LTTNG\n> #include \n> +#endif /* HAVE_LTTNG */\n> \n> {{source}}\n> \n> #endif /* INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H */\n\nSame for this.\n\nI wonder if it's easier just to have a top-level `if HAVE_LTTNG and if\nHAVE_PERFETTO` and just duplicate {{source}}. It's only one line that's\nduplicated anyway.\n\n> \n> +#if HAVE_LTTNG\n> #include \n> +#endif /* HAVE_LTTNG */\n> \n> -#endif /* HAVE_TRACING */\n> +#endif /* HAVE_PERFETTO || HAVE_LTTNG */\n> diff --git a/include/libcamera/internal/tracepoints/meson.build b/include/libcamera/internal/tracepoints/meson.build\n> index d9b2fca5..ff5aece6 100644\n> --- a/include/libcamera/internal/tracepoints/meson.build\n> +++ b/include/libcamera/internal/tracepoints/meson.build\n> @@ -1,12 +1,19 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> -# enum files must go first\n> -tracepoint_files = files([\n> - 'buffer_enums.tp',\n> - 'request_enums.tp',\n> -])\n> +if get_option('android').enabled() and get_option('android_platform') == 'cros'\n> + tracepoint_files = files([\n> + 'pipeline.perfetto',\n> + 'request.perfetto',\n> + ])\n> +else\n> + # enum files must go first\n> + tracepoint_files = files([\n> + 'buffer_enums.tp',\n> + 'request_enums.tp',\n> + ])\n> \n> -tracepoint_files += files([\n> - 'pipeline.tp',\n> - 'request.tp',\n> -])\n> + tracepoint_files += files([\n> + 'pipeline.tp',\n> + 'request.tp',\n> + ])\n> +endif\n\nAnd this is going to need a bit more moving parts for code generation...\n\n> diff --git a/include/libcamera/internal/tracepoints/pipeline.perfetto b/include/libcamera/internal/tracepoints/pipeline.perfetto\n> new file mode 100644\n> index 00000000..5f45295e\n> --- /dev/null\n> +++ b/include/libcamera/internal/tracepoints/pipeline.perfetto\n> @@ -0,0 +1,10 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * pipeline.tp - Tracepoints for pipelines\n> + */\n> +\n> +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char *func);\n> +\n> +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char *func);\n\nBasically, we already have definitions for these tracepoints in\ninclude/libcamera/internal/tracepoints/pipeline.tp. It's obviously not a\ngood idea to have duplicate definitions.\n\nSo we either need to use lttng's tracepoints definitions to generate\nperfetto's here, or we create a new language from which we generate\ntracepoints for both. imo the latter is overkill (and costs a lot more\neffort) so probably the former is better.\n\nI'll use the request tracepoints as examples below just because they\nhave a bit more beef than these pipeline tracepoints which are just two\nstrings.\n\n> diff --git a/include/libcamera/internal/tracepoints/request.perfetto b/include/libcamera/internal/tracepoints/request.perfetto\n> new file mode 100644\n> index 00000000..fd6a42a4\n> --- /dev/null\n> +++ b/include/libcamera/internal/tracepoints/request.perfetto\n> @@ -0,0 +1,30 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * request.tp - Tracepoints for the request object\n> + */\n> +\n> +#include \n> +\n> +#include \n> +\n> +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req);\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req);\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req);\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req);\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req);\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request *req);\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private *req);\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private *req);\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(\n> +\t\tlibcamera::Request::Private *req,\n> +\t\tlibcamera::FrameBuffer * buf);\n> diff --git a/meson.build b/meson.build\n> index e4c0be16..79514b5b 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -156,7 +156,11 @@ libcamera_includes = include_directories('include')\n> py_modules = []\n> \n> # Libraries used by multiple components\n> -liblttng = dependency('lttng-ust', required : get_option('tracing'))\n> +libperfetto = dependency('perfetto', required : get_option('tracing').enabled()\n> + and get_option('android').enabled()\n> + and get_option('android_platform') == 'cros')\n> +liblttng = cc.find_library('lttng-ust', required : get_option('tracing').enabled()\n> + and not libperfetto.found())\n> \n> # Pipeline handlers\n> #\n> @@ -208,6 +212,7 @@ py_mod.find_installation('python3', modules: py_modules)\n> summary({\n> 'Enabled pipelines': pipelines,\n> 'Enabled IPA modules': enabled_ipa_modules,\n> + 'Perfetto support': perfetto_enabled,\n> 'Tracing support': tracing_enabled,\n> 'Android support': android_enabled,\n> 'GStreamer support': gst_enabled,\n> diff --git a/src/android/cros/camera3_hal.cpp b/src/android/cros/camera3_hal.cpp\n> index fb863b5f..2fbd7f14 100644\n> --- a/src/android/cros/camera3_hal.cpp\n> +++ b/src/android/cros/camera3_hal.cpp\n> @@ -7,10 +7,15 @@\n> \n> #include \n> \n> +#include \"libcamera/internal/tracepoints.h\"\n> #include \"../camera_hal_manager.h\"\n> \n> static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken *token)\n> {\n> +\tperfetto::TracingInitArgs args;\n> +\targs.backends |= perfetto::kSystemBackend;\n> +\tperfetto::Tracing::Initialize(args);\n> +\tperfetto::TrackEvent::Register();\n> }\n> \n> static void tear_down()\n> diff --git a/src/android/cros/meson.build b/src/android/cros/meson.build\n> index 35995dd8..68f2bd9e 100644\n> --- a/src/android/cros/meson.build\n> +++ b/src/android/cros/meson.build\n> @@ -9,5 +9,6 @@ android_hal_sources += files([\n> ])\n> \n> android_deps += dependency('libcros_camera')\n> +android_deps += dependency('perfetto')\n> \n> android_cpp_args += ['-DOS_CHROMEOS']\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 0494e808..ba8bf26e 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -62,6 +62,7 @@ libthreads = dependency('threads')\n> \n> subdir('base')\n> subdir('ipa')\n> +subdir('perfetto')\n> subdir('pipeline')\n> subdir('proxy')\n> \n> @@ -89,11 +90,19 @@ if not libcrypto.found()\n> warning('Neither gnutls nor libcrypto found, all IPA modules will be isolated')\n> endif\n> \n> -if liblttng.found()\n> +if libperfetto.found()\n> + perfetto_enabled = true\n> + tracing_enabled = false\n> + config_h.set('HAVE_PERFETTO', 1)\n> + libcamera_sources += perfetto_sources\n> + libcamera_sources += files(['tracepoints.cpp'])\n> +elif liblttng.found()\n> + perfetto_enabled = false\n> tracing_enabled = true\n> - config_h.set('HAVE_TRACING', 1)\n> + config_h.set('HAVE_LTTNG', 1)\n> libcamera_sources += files(['tracepoints.cpp'])\n> else\n> + perfetto_enabled = false\n> tracing_enabled = false\n> endif\n> \n> @@ -154,6 +163,7 @@ libcamera_deps = [\n> libcrypto,\n> libdl,\n> liblttng,\n> + libperfetto,\n> libudev,\n> libyaml,\n> ]\n> diff --git a/src/libcamera/perfetto/meson.build b/src/libcamera/perfetto/meson.build\n> new file mode 100644\n> index 00000000..119a1ad7\n> --- /dev/null\n> +++ b/src/libcamera/perfetto/meson.build\n> @@ -0,0 +1,6 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +perfetto_sources = files([\n> + 'pipeline_perfetto.cpp',\n> + 'request_perfetto.cpp',\n> +])\n> diff --git a/src/libcamera/perfetto/pipeline_perfetto.cpp b/src/libcamera/perfetto/pipeline_perfetto.cpp\n> new file mode 100644\n> index 00000000..07b82ffd\n> --- /dev/null\n> +++ b/src/libcamera/perfetto/pipeline_perfetto.cpp\n> @@ -0,0 +1,24 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * pipeline.tp - Tracepoints for pipelines\n> + */\n> +\n> +#include \"libcamera/internal/tracepoints.h\"\n> +\n> +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char *func)\n> +{\n> +\t// TODO: Consider TRACE_EVENT_BEGIN\n\nFrom what I've read, I think TRACE_EVENT_BEGIN is better.\n\nIn the perfetto tracepoints generator we could automatically detect\n_begin and _end suffixes and convert those to TRACE_EVENT_BEGIN and\nTRACE_EVENT_END.\n\n> +\tTRACE_EVENT(\"libcamera\", \"ipa_call_begin\",\n> +\t\t \"pipeline_name\", pipe,\n> +\t\t \"function_name\", func);\n> +}\n> +\n> +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char *func)\n> +{\n> +\t// TODO: Consider TRACE_EVENT_END\n> +\tTRACE_EVENT(\"libcamera\", \"ipa_call_end\",\n> +\t\t \"pipeline_name\", pipe,\n> +\t\t \"function_name\", func);\n> +}\n> diff --git a/src/libcamera/perfetto/request_perfetto.cpp b/src/libcamera/perfetto/request_perfetto.cpp\n> new file mode 100644\n> index 00000000..2cfff28e\n> --- /dev/null\n> +++ b/src/libcamera/perfetto/request_perfetto.cpp\n> @@ -0,0 +1,73 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2022, Google Inc.\n> + *\n> + * request.tp - Tracepoints for the request object\n> + */\n> +\n> +#include \n> +\n> +#include \"libcamera/internal/request.h\"\n> +#include \"libcamera/internal/tracepoints.h\"\n> +\n> +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request\",\n> +\t\t \"request_ptr\", reinterpret_cast(req),\n> +\t\t \"cookie\", req->cookie(),\n> +\t\t \"status\", req->status()); // TODO\n\n(What is this TODO for?)\n\n> +}\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request_construct\",\n> +\t\t \"request_ptr\", reinterpret_cast(req));\n> +}\n\nSo in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments\nand fields and how to obtain the fields from the args. We can\ninstantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only\nhave to specify the args, and the fields are already defined and come\nfrom the class. This reduces duplication in the tracepoint definitions.\n\nI'm wondering from what you've implemented here that perfetto doesn't\nsupport this? I don't see anything in the perfetto docs that suggest\nthat this is possible either. Although we can mitigate this issue by\ngenerating the perfetto tracepoints :) (this is another reason why I\nthink we should use the lttng tracepoints definition as the main one and\ngenerate the perfetto ones from those)\n\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request_destroy\",\n> +\t\t \"request_ptr\", reinterpret_cast(req));\n> +}\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request_reuse\",\n> +\t\t \"request_ptr\", reinterpret_cast(req));\n> +}\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request_queue\",\n> +\t\t \"request_ptr\", reinterpret_cast(req));\n> +}\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request *req)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request_device_queue\",\n> +\t\t \"request_ptr\", reinterpret_cast(req));\n> +}\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private *req)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request_complete\",\n> +\t\t \"request_private_ptr\", reinterpret_cast(req));\n> +}\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private *req)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request_cancel\",\n> +\t\t \"request_private_ptr\", reinterpret_cast(req));\n> +}\n> +\n> +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(\n> +\tlibcamera::Request::Private *req,\n> +\tlibcamera::FrameBuffer *buf)\n> +{\n> +\tTRACE_EVENT(\"libcamera\", \"request_complete_buffer\",\n> +\t\t \"request_private_ptr\", reinterpret_cast(req),\n> +\t\t \"cookie\", req->_o()->cookie(),\n> +\t\t \"status\", req->_o()->status(), // TODO\n> +\t\t \"buffer_ptr\", reinterpret_cast(buf),\n> +\t\t \"buffer_status\", buf->metadata().status); // TODO\n> +}\n\nAlright, I'm going to use this as an example.\n\nFirst, the tracepoint definition. In lttng we have:\n\nTRACEPOINT_EVENT(\n\tlibcamera,\n\trequest_complete_buffer,\n\tTP_ARGS(\n\t\tlibcamera::Request::Private *, req,\n\t\tlibcamera::FrameBuffer *, buf\n\t),\n\tTP_FIELDS(\n\t\tctf_integer_hex(uintptr_t, request, reinterpret_cast(req))\n\t\tctf_integer(uint64_t, cookie, req->_o()->cookie())\n\t\tctf_integer(int, status, req->_o()->status())\n\t\tctf_integer_hex(uintptr_t, buffer, reinterpret_cast(buf))\n\t\tctf_enum(libcamera, buffer_status, uint32_t, buf_status, buf->metadata().status)\n\t)\n)\n\nAnd to raise (idk the right verb) the tracepoint, we use\ntracepoint(libcamera, request_complete_buffer, request_ptr, buffer_ptr).\nIn perfetto as far as I understand, it's raised using TRACE_EVENT(...)\nand you have to specify everything that lttng wraps away in TP_FIELDS in\nthe tracepoint definition itself. For this reason, I think it's good to\nhave the intermediary functions.\n\nThe idea is to generate both the perfetto tracepoint definition and the\nintermediary function from the lttng definition. I've studied the\nperfetto tracepoint definition docs a bit more than last time, and I\nthink that going from lttng's definitions to perfetto's is better.\n\nI was going to walk through an example, but I'm not sure how because it\nseems pretty straightforward to me. We can parse the TRACEPOINT_EVENT,\nand extract the category name and tracepoint name into the function name\nand TRACE_EVENT parameters. TP_ARGS also can get parsed and copied into\nthe function parameters almost as-is. TP_FIELDS needs a bit of\ntransformation, but it's just extracting the field name and\ntransformation, and then adding an extra cast for the type. ctf_enum\nwould be a bit more involved though, but we have definitions in .tp\nfiles for the enums that map the integer values to strings, so we can\nuse that. It might involve generating an intermediate file for all the\nenums, or I guess we could just generate enum definitions and embed that\nin the perfetto tracepoint function definitions cpp file.\n\nFor TRACEPOINT_EVENT_CLASS and TRACEPOINT_EVENT_INSTANCE there's just\none extra level of indirection to obtain TP_FIELDS, but otherwise we\njust match it based on the tracepoint class name.\n\nAs for \"how\", we can use either ply or a custom-made simple C parser for\nparsing, and then we'd have to make a custom code generator. If we go\nhomemade C parser route it doesn't have to be too complex, as we only\nhave TRACEPOINT_EVENT, TRACEPOINT_EVENT_CLASS, and\nTRACEPOINT_EVENT_INSTANCE to parse, and the complex expressions are\nsimply copied. And then for the code generation part we can use jinja2.\nimo the most difficult part is plumbing it through meson, but we've done\nthat before in the IPC layer, so that has examples of how to do code\ngeneration -> compilation via meson [2] [3] [4], as well as examples for\njinja2 (although it's actually embedded in mojom).\n\n[2] utils/ipc/meson.build\n[3] include/libcamera/ipa/meson.build\n[4] src/libcamera/ipa/meson.build\n\nWould you be able to do this?\n\n\nPaul\n\n> diff --git a/src/libcamera/tracepoints.cpp b/src/libcamera/tracepoints.cpp\n> index 0173b75a..a07ea531 100644\n> --- a/src/libcamera/tracepoints.cpp\n> +++ b/src/libcamera/tracepoints.cpp\n> @@ -4,7 +4,18 @@\n> *\n> * tracepoints.cpp - Tracepoints with lttng\n> */\n> +\n> +#if HAVE_PERFETTO\n> +\n> +#include \"libcamera/internal/tracepoints.h\"\n> +\n> +PERFETTO_TRACK_EVENT_STATIC_STORAGE();\n> +\n> +#else\n> +\n> #define TRACEPOINT_CREATE_PROBES\n> #define TRACEPOINT_DEFINE\n> \n> #include \"libcamera/internal/tracepoints.h\"\n> +\n> +#endif /* HAVE_PERFETTO */\n> -- \n> 2.39.0.314.g84b9a713c41-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9E158BD78E\n\tfor ;\n\tTue, 27 Dec 2022 04:09:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C764A625CF;\n\tTue, 27 Dec 2022 05:09:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1762761F0E\n\tfor ;\n\tTue, 27 Dec 2022 05:09:06 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8EEB2514;\n\tTue, 27 Dec 2022 05:09:04 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1672114147;\n\tbh=/YhPZI4q88ZFI9rq3/EU03LHoeDG0003IhNjOJY6TPc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=t9c7R58ajHH6lQInngOxgyV/DRcuLzNkP7SPSNRZ0qDuaXcSyT/jXTgi05WOaT5D5\n\tF6OzN2ckK9G+sj7uaRYvqych+WLWalDw2BSZ7Tf3GAdS7Pjb2JkXDmYcSpJiLjebRe\n\tZ1YGE4vzK9QzTW9Sow/+3CN/iYQqJgOqrpvF/dZyFVhovwn34NSJMMAy6coOetYsel\n\t6T2/Aku7txAK4tmrIFPubzsrfCrpNbcIECJfXfATZjYhmJiFEGv2CrVeB5v6UJozJE\n\tIeggb3W2Zm8SMgXyH8ihM0Q9tBBBNV/zUUQbCA+Or1vXpHun3bqjLs45NOh3wjldhc\n\twosNSTINHJxFA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1672114145;\n\tbh=/YhPZI4q88ZFI9rq3/EU03LHoeDG0003IhNjOJY6TPc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pzBs5fjC57FqmKMjxILTNEEOKYS+UTbMzzmKOvwQ8ZgBLXG5ffxcRknGAOh4g4LUW\n\tgwbNuxEBakpxpxTVrHfm8+msb37vH4xh8pBMbqsIRM2zGC94SSY1kE8dkMTz1HXIp/\n\tFPdDciUy5qYEyZjZT52vdCp5kVzlQ4MqLfz0mNYM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pzBs5fjC\"; dkim-atps=neutral","Date":"Mon, 26 Dec 2022 22:08:57 -0600","To":"Harvey Yang ","Message-ID":"","References":"<20221220070523.3212844-1-chenghaoyang@google.com>\n\t<20221220070523.3212844-2-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20221220070523.3212844-2-chenghaoyang@google.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30502,"web_url":"https://patchwork.libcamera.org/comment/30502/","msgid":"","date":"2024-07-30T13:30:38","subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Paul,\n\nSorry for the late reply. I've been working on mtkisp7's migration :p\n\nOn Tue, Dec 27, 2022 at 12:09 PM Paul Elder \nwrote:\n\n> Hi Harvey,\n>\n> On Tue, Dec 20, 2022 at 07:05:23AM +0000, Harvey Yang via libcamera-devel\n> wrote:\n> > As ChromeOS is using perfetto (project named as CrOSetto). When ChromeOS\n> > uses libcamera, it should use perfetto to collect traces instead of\n> > lttng.\n>\n> I'll start with the elephant in the room: we cannot have duplicate\n> tracepoint definitions. There must be only one definition of the\n> tracepoints, and either one or both of perfetto's and lttng's\n> tracepoints must be generated from that.\n>\n> For instance, I've already got a series pending [1] that expands on the\n> request tracepoints quite a bit; it's clearly unscalable to deal with\n> two duplicate sets of tracepoints (I'm not sure why I have to defend\n> deduplication, but anyway just trying to be clear about my stance).\n>\n> [1] https://patchwork.libcamera.org/project/libcamera/list/?series=3668\n>\n> More on this in the relevant section in the patch.\n>\n> >\n> > Signed-off-by: Harvey Yang \n> > ---\n> > Documentation/guides/tracing.rst | 125 ++++++++++++++----\n> > include/libcamera/internal/tracepoints.h.in | 38 +++++-\n> > .../internal/tracepoints/meson.build | 25 ++--\n> > .../internal/tracepoints/pipeline.perfetto | 10 ++\n> > .../internal/tracepoints/request.perfetto | 30 +++++\n> > meson.build | 7 +-\n> > src/android/cros/camera3_hal.cpp | 5 +\n> > src/android/cros/meson.build | 1 +\n> > src/libcamera/meson.build | 14 +-\n> > src/libcamera/perfetto/meson.build | 6 +\n> > src/libcamera/perfetto/pipeline_perfetto.cpp | 24 ++++\n> > src/libcamera/perfetto/request_perfetto.cpp | 73 ++++++++++\n> > src/libcamera/tracepoints.cpp | 11 ++\n> > 13 files changed, 325 insertions(+), 44 deletions(-)\n> > create mode 100644\n> include/libcamera/internal/tracepoints/pipeline.perfetto\n> > create mode 100644\n> include/libcamera/internal/tracepoints/request.perfetto\n> > create mode 100644 src/libcamera/perfetto/meson.build\n> > create mode 100644 src/libcamera/perfetto/pipeline_perfetto.cpp\n> > create mode 100644 src/libcamera/perfetto/request_perfetto.cpp\n> >\n> > diff --git a/Documentation/guides/tracing.rst\n> b/Documentation/guides/tracing.rst\n> > index ae960d85..bd5b3453 100644\n> > --- a/Documentation/guides/tracing.rst\n> > +++ b/Documentation/guides/tracing.rst\n> > @@ -16,33 +16,50 @@ at periodic points in time. This can be done with\n> other tools such as\n> > callgrind, perf, gprof, etc., without modification to the application,\n> > and is out of scope for this guide.\n> >\n> > +Library: Perfetto vs lttng-ust\n> > +------------------------------\n> > +\n> > +To integrate with CrOS tracing infrastructure, which uses perfetto (or\n> called\n>\n> The first instance of \"CrOS\" should probably be spelled out.\n>\n> > +CrOSetto in CrOS), we implement the tracepoint macros (will be\n> described below)\n> > +with two different libraries: CrOS with perfetto, and the rest with\n> lttng-ust.\n> > +\n> > Compiling\n> > ---------\n> >\n> > -To compile libcamera with tracing support, it must be enabled through\n> the\n> > -meson ``tracing`` option. It depends on the lttng-ust library\n> (available in the\n> > -``liblttng-ust-dev`` package for Debian-based distributions).\n> > -By default the tracing option in meson is set to ``auto``, so if\n> > -liblttng is detected, it will be enabled by default. Conversely, if the\n> option\n> > -is set to disabled, then libcamera will be compiled without tracing\n> support.\n> > +To compile libcamera with tracing support in CrOS, it must be enabled\n> through\n>\n> s/ in CrOS//\n>\n> > +the meson ``tracing`` option. In CrOS, it depends on the perfetto\n> library,\n> > +which is by default available in CrOS; In the rest, it depends on the\n> lttng-ust\n>\n> s/;/./\n>\n> s/the rest/non-CrOS/\n>\n> > +library (available in the ``liblttng-ust-dev`` package for Debian-based\n> > +distributions).\n> > +By default the tracing option in meson is set to ``auto``, so if the\n> library is\n> > +detected, it will be enabled by default. Conversely, if the option is\n> set to\n> > +disabled, then libcamera will be compiled without tracing support.\n> >\n> > Defining tracepoints\n> > --------------------\n> >\n> > libcamera already contains a set of tracepoints. To define additional\n> > -tracepoints, create a file\n> > -``include/libcamera/internal/tracepoints/{file}.tp``, where ``file`` is\n> a\n> > -reasonable name related to the category of tracepoints that you wish to\n> > -define. For example, the tracepoints file for the Request object is\n> called\n> > -``request.tp``. An entry for this file must be added in\n> > -``include/libcamera/internal/tracepoints/meson.build``.\n> > -\n> > -In this tracepoints file, define your tracepoints `as mandated by lttng\n> > +tracepoints, create two files ``{file}.tp`` and ``{file}.perfetto``,\n> for lttng\n> > +and perfetto respectively, under\n> ``include/libcamera/internal/tracepoints/``,\n> > +and the perfetto implementation file\n> ``src/libcamera/{file}_perfetto.cpp``,\n> > +where ``file`` is a reasonable name related to the category of\n> tracepoints that\n> > +you wish to define. For example, the tracepoints files for the Request\n> object is\n> > +called ``request.tp`` and ``request.perfetto``. Entries for the files\n> must be\n> > +added in ``include/libcamera/internal/tracepoints/meson.build``.\n> > +\n> > +In the perfetto tracepoints files, declare the tracepoint functions in\n> > +``{file}.perfetto``, and define them in ``{file}_perfetto.cpp``, `as\n> mandated\n> > +by perfetto `_.\n> > +Currently the only enabled perfetto::Category is ``libcamera``. If you\n> intend to\n> > +use another category, remember to define it in\n> > +``include/libcamera/internal/tracepoints.h.in``, and enable it when\n> collecting\n> > +traces.\n>\n> These sections will have to be fixed.\n>\n> > +\n> > +In the lttng tracepoints file, define your tracepoints `as mandated by\n> lttng\n> > `_. The header boilerplate must\n> *not* be\n> > included (as it will conflict with the rest of our infrastructure), and\n> > only the tracepoint definitions (with the ``TRACEPOINT_*`` macros)\n> should be\n> > included.\n> > -\n> > All tracepoint providers shall be ``libcamera``. According to lttng, the\n> > tracepoint provider should be per-project; this is the rationale for\n> this\n> > decision. To group tracepoint events, we recommend using\n> > @@ -68,9 +85,9 @@ Then to use the tracepoint:\n> >\n> > ``LIBCAMERA_TRACEPOINT({tracepoint_event}, args...)``\n> >\n> > -This macro must be used, as opposed to lttng's macros directly, because\n> > -lttng is an optional dependency of libcamera, so the code must compile\n> and run\n> > -even when lttng is not present or when tracing is disabled.\n> > +This macro must be used, as opposed to perfetto's or lttng's macros\n> directly,\n> > +because tracing support is optional of libcamera, so the code must\n> compile and\n>\n> s/of/in/\n>\n> > +run even when perfetto or lttng are not present or when tracing is\n> disabled.\n> >\n> > The tracepoint provider name, as declared in the tracepoint definition,\n> is not\n> > included in the parameters of the tracepoint.\n> > @@ -87,21 +104,75 @@ respectively. These are the tracepoints that our\n> sample analysis script\n> > (see \"Analyzing a trace\") scans for when computing statistics on IPA\n> call time.\n> >\n> > Using tracepoints (from an application)\n> > ----------------------------------------\n> > +---------------------------------------------\n> >\n> > As applications are not part of libcamera, but rather users of\n> libcamera,\n> > applications should seek their own tracing mechanisms. For ease of\n> tracing\n> > the application alongside tracing libcamera, it is recommended to also\n> > -`use lttng <\n> https://lttng.org/docs/#doc-tracing-your-own-user-application>`_.\n> > +`use lttng <\n> https://lttng.org/docs/#doc-tracing-your-own-user-application>`_,\n> > +or `use perfetto >`_.\n> >\n> > Using tracepoints (from closed-source IPA)\n> > -------------------------------------------\n> > +------------------------------------------------\n>\n> Underline length.\n>\n> > +\n> > +Similar to applications, closed-source IPAs can simply use perfetto or\n> lttng\n> > +on their own, or any other tracing mechanism if desired.\n> > +\n> > +Collecting a perfetto trace\n> > +---------------------------\n> > +\n> > +A trace can be collected with the following steps:\n> > +\n> > +1. Start `traced` if it hasn't been started.\n> > +.. code-block:: bash\n> > +\n> > + start traced\n> > +\n> > +2. Start a consumer that includes “track_event” data source in the trace\n> > + config, and “libcamera” category.\n> > +.. code-block:: bash\n> > +\n> > + perfetto -c - --txt -o /tmp/perfetto-trace \\\n>\n> (Reply to your comment in the other email) I think this is sufficient,\n> as apparently the option for a config file is documented in the help\n> text.\n>\n> > + < > +\n> > + buffers: {\n> > + size_kb: 63488\n> > + fill_policy: DISCARD\n> > + }\n> > + buffers: {\n> > + size_kb: 2048\n> > + fill_policy: DISCARD\n> > + }\n> > + data_sources: {\n> > + config {\n> > + name: \"track_event\"\n> > + track_event_config {\n> > + enabled_categories: \"libcamera\"\n> > + }\n> > + }\n> > + }\n> > + duration_ms: 10000\n> > +\n> > + EOF\n> > +\n> > +3. Execute the libcamera behavior you intend to trace during the set\n> duration\n> > + (10000 ms) in the above example.\n> > +\n> > +4. After the consumer (cmd `perfetto`) is done, you can find the trace\n> result\n> > + in ``/tmp/perfetto-trace``.\n> > +\n> > +Analyzing a perfetto trace\n> > +--------------------------\n> > +\n> > +Follow the `guide `_\n> to\n> > +visualize the trace.\n> >\n> > -Similar to applications, closed-source IPAs can simply use lttng on\n> their own,\n> > -or any other tracing mechanism if desired.\n> > +In other words, upload the trace to `Perfetto UI <\n> https://ui.perfetto.dev/>`_,\n> > +where you can check the timeline of tracepoints on each process/thread.\n> You can\n> > +also run SQL queries to do analysis.\n> >\n> > -Collecting a trace\n> > -------------------\n> > +Collecting an lttng trace\n> > +------------------------\n>\n> Underline length.\n>\n> >\n> > A trace can be collected fairly simply from lttng:\n> >\n> > @@ -123,8 +194,8 @@ viewed by: ``lttng view -t $PATH_TO_TRACE``, where\n> ``$PATH_TO_TRACE`` is the\n> > path that was printed when the session was created. This is the same\n> path that\n> > is used when analyzing traces programatically, as described in the next\n> section.\n> >\n> > -Analyzing a trace\n> > ------------------\n> > +Analyzing a lttng trace\n>\n> s/a/an/\n>\n> > +-----------------------\n> >\n> > As mentioned above, while an lttng tracing session exists and the trace\n> is not\n> > running, the trace output can be viewed as text by ``lttng view``.\n> > diff --git a/include/libcamera/internal/tracepoints.h.in\n> b/include/libcamera/internal/tracepoints.h.in\n> > index d0fc1365..b6c4abea 100644\n> > --- a/include/libcamera/internal/tracepoints.h.in\n> > +++ b/include/libcamera/internal/tracepoints.h.in\n> > @@ -9,7 +9,24 @@\n> > #ifndef __LIBCAMERA_INTERNAL_TRACEPOINTS_H__\n> > #define __LIBCAMERA_INTERNAL_TRACEPOINTS_H__\n> >\n> > -#if HAVE_TRACING\n> > +#if HAVE_PERFETTO\n> > +\n> > +#include \n> > +\n> > +PERFETTO_DEFINE_CATEGORIES(\n> > + perfetto::Category(\"libcamera\")\n> > + .SetDescription(\"Events from libcamera\"));\n> > +\n> > +#define LIBCAMERA_TRACEPOINT(t_name, ...) \\\n> > +LIBCAMERA_TRACE_EVENT_##t_name(__VA_ARGS__)\n> > +\n> > +#define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \\\n> > +LIBCAMERA_TRACE_EVENT_ipa_call_begin(#pipe, #func)\n> > +\n> > +#define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \\\n> > +LIBCAMERA_TRACE_EVENT_ipa_call_end(#pipe, #func)\n> > +\n> > +#elif HAVE_LTTNG /* !HAVE_PERFETTO */\n> > #define LIBCAMERA_TRACEPOINT(...) tracepoint(libcamera, __VA_ARGS__)\n> >\n> > #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \\\n> > @@ -18,7 +35,7 @@ tracepoint(libcamera, ipa_call_begin, #pipe, #func)\n> > #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \\\n> > tracepoint(libcamera, ipa_call_end, #pipe, #func)\n> >\n> > -#else\n> > +#else /* !HAVE_PERFETTO && !HAVE_LTTNG */\n> >\n> > namespace {\n> >\n> > @@ -34,12 +51,17 @@ inline void unused([[maybe_unused]] Args&& ...args)\n> > #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func)\n> > #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func)\n> >\n> > -#endif /* HAVE_TRACING */\n> > +#endif /* HAVE_PERFETTO */\n> >\n> > #endif /* __LIBCAMERA_INTERNAL_TRACEPOINTS_H__ */\n> >\n> > +#if HAVE_PERFETTO || HAVE_LTTNG\n> > +\n> > +#if HAVE_PERFETTO\n> >\n> > -#if HAVE_TRACING\n> > +#include \n> > +\n> > +#else /* HAVE_LTTNG */\n> >\n> > #undef TRACEPOINT_PROVIDER\n> > #define TRACEPOINT_PROVIDER libcamera\n> > @@ -47,15 +69,21 @@ inline void unused([[maybe_unused]] Args&& ...args)\n> > #undef TRACEPOINT_INCLUDE\n> > #define TRACEPOINT_INCLUDE \"{{path}}\"\n> >\n> > +#endif /* HAVE_PERFETTO */\n> > +\n> > #if !defined(INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H) ||\n> defined(TRACEPOINT_HEADER_MULTI_READ)\n> > #define INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H\n>\n> I'm pretty sure #endif /* HAVE_PERFETTO */ should come here (unless\n> perfetto also needs this? Does it?).\n>\n> >\n> > +#if HAVE_LTTNG\n> > #include \n> > +#endif /* HAVE_LTTNG */\n> >\n> > {{source}}\n> >\n> > #endif /* INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H */\n>\n> Same for this.\n>\n> I wonder if it's easier just to have a top-level `if HAVE_LTTNG and if\n> HAVE_PERFETTO` and just duplicate {{source}}. It's only one line that's\n> duplicated anyway.\n>\n> >\n> > +#if HAVE_LTTNG\n> > #include \n> > +#endif /* HAVE_LTTNG */\n> >\n> > -#endif /* HAVE_TRACING */\n> > +#endif /* HAVE_PERFETTO || HAVE_LTTNG */\n> > diff --git a/include/libcamera/internal/tracepoints/meson.build\n> b/include/libcamera/internal/tracepoints/meson.build\n> > index d9b2fca5..ff5aece6 100644\n> > --- a/include/libcamera/internal/tracepoints/meson.build\n> > +++ b/include/libcamera/internal/tracepoints/meson.build\n> > @@ -1,12 +1,19 @@\n> > # SPDX-License-Identifier: CC0-1.0\n> >\n> > -# enum files must go first\n> > -tracepoint_files = files([\n> > - 'buffer_enums.tp',\n> > - 'request_enums.tp',\n> > -])\n> > +if get_option('android').enabled() and get_option('android_platform')\n> == 'cros'\n> > + tracepoint_files = files([\n> > + 'pipeline.perfetto',\n> > + 'request.perfetto',\n> > + ])\n> > +else\n> > + # enum files must go first\n> > + tracepoint_files = files([\n> > + 'buffer_enums.tp',\n> > + 'request_enums.tp',\n> > + ])\n> >\n> > -tracepoint_files += files([\n> > - 'pipeline.tp',\n> > - 'request.tp',\n> > -])\n> > + tracepoint_files += files([\n> > + 'pipeline.tp',\n> > + 'request.tp',\n> > + ])\n> > +endif\n>\n> And this is going to need a bit more moving parts for code generation...\n>\n> > diff --git a/include/libcamera/internal/tracepoints/pipeline.perfetto\n> b/include/libcamera/internal/tracepoints/pipeline.perfetto\n> > new file mode 100644\n> > index 00000000..5f45295e\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/tracepoints/pipeline.perfetto\n> > @@ -0,0 +1,10 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * pipeline.tp - Tracepoints for pipelines\n> > + */\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char\n> *func);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char\n> *func);\n>\n> Basically, we already have definitions for these tracepoints in\n> include/libcamera/internal/tracepoints/pipeline.tp. It's obviously not a\n> good idea to have duplicate definitions.\n>\n> So we either need to use lttng's tracepoints definitions to generate\n> perfetto's here, or we create a new language from which we generate\n> tracepoints for both. imo the latter is overkill (and costs a lot more\n> effort) so probably the former is better.\n>\n> I'll use the request tracepoints as examples below just because they\n> have a bit more beef than these pipeline tracepoints which are just two\n> strings.\n>\n> > diff --git a/include/libcamera/internal/tracepoints/request.perfetto\n> b/include/libcamera/internal/tracepoints/request.perfetto\n> > new file mode 100644\n> > index 00000000..fd6a42a4\n> > --- /dev/null\n> > +++ b/include/libcamera/internal/tracepoints/request.perfetto\n> > @@ -0,0 +1,30 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * request.tp - Tracepoints for the request object\n> > + */\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request\n> *req);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private\n> *req);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private\n> *req);\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(\n> > + libcamera::Request::Private *req,\n> > + libcamera::FrameBuffer * buf);\n> > diff --git a/meson.build b/meson.build\n> > index e4c0be16..79514b5b 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -156,7 +156,11 @@ libcamera_includes = include_directories('include')\n> > py_modules = []\n> >\n> > # Libraries used by multiple components\n> > -liblttng = dependency('lttng-ust', required : get_option('tracing'))\n> > +libperfetto = dependency('perfetto', required :\n> get_option('tracing').enabled()\n> > + and get_option('android').enabled()\n> > + and get_option('android_platform') == 'cros')\n> > +liblttng = cc.find_library('lttng-ust', required :\n> get_option('tracing').enabled()\n> > + and not libperfetto.found())\n> >\n> > # Pipeline handlers\n> > #\n> > @@ -208,6 +212,7 @@ py_mod.find_installation('python3', modules:\n> py_modules)\n> > summary({\n> > 'Enabled pipelines': pipelines,\n> > 'Enabled IPA modules': enabled_ipa_modules,\n> > + 'Perfetto support': perfetto_enabled,\n> > 'Tracing support': tracing_enabled,\n> > 'Android support': android_enabled,\n> > 'GStreamer support': gst_enabled,\n> > diff --git a/src/android/cros/camera3_hal.cpp\n> b/src/android/cros/camera3_hal.cpp\n> > index fb863b5f..2fbd7f14 100644\n> > --- a/src/android/cros/camera3_hal.cpp\n> > +++ b/src/android/cros/camera3_hal.cpp\n> > @@ -7,10 +7,15 @@\n> >\n> > #include \n> >\n> > +#include \"libcamera/internal/tracepoints.h\"\n> > #include \"../camera_hal_manager.h\"\n> >\n> > static void set_up([[maybe_unused]] cros::CameraMojoChannelManagerToken\n> *token)\n> > {\n> > + perfetto::TracingInitArgs args;\n> > + args.backends |= perfetto::kSystemBackend;\n> > + perfetto::Tracing::Initialize(args);\n> > + perfetto::TrackEvent::Register();\n> > }\n> >\n> > static void tear_down()\n> > diff --git a/src/android/cros/meson.build b/src/android/cros/meson.build\n> > index 35995dd8..68f2bd9e 100644\n> > --- a/src/android/cros/meson.build\n> > +++ b/src/android/cros/meson.build\n> > @@ -9,5 +9,6 @@ android_hal_sources += files([\n> > ])\n> >\n> > android_deps += dependency('libcros_camera')\n> > +android_deps += dependency('perfetto')\n> >\n> > android_cpp_args += ['-DOS_CHROMEOS']\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index 0494e808..ba8bf26e 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -62,6 +62,7 @@ libthreads = dependency('threads')\n> >\n> > subdir('base')\n> > subdir('ipa')\n> > +subdir('perfetto')\n> > subdir('pipeline')\n> > subdir('proxy')\n> >\n> > @@ -89,11 +90,19 @@ if not libcrypto.found()\n> > warning('Neither gnutls nor libcrypto found, all IPA modules will\n> be isolated')\n> > endif\n> >\n> > -if liblttng.found()\n> > +if libperfetto.found()\n> > + perfetto_enabled = true\n> > + tracing_enabled = false\n> > + config_h.set('HAVE_PERFETTO', 1)\n> > + libcamera_sources += perfetto_sources\n> > + libcamera_sources += files(['tracepoints.cpp'])\n> > +elif liblttng.found()\n> > + perfetto_enabled = false\n> > tracing_enabled = true\n> > - config_h.set('HAVE_TRACING', 1)\n> > + config_h.set('HAVE_LTTNG', 1)\n> > libcamera_sources += files(['tracepoints.cpp'])\n> > else\n> > + perfetto_enabled = false\n> > tracing_enabled = false\n> > endif\n> >\n> > @@ -154,6 +163,7 @@ libcamera_deps = [\n> > libcrypto,\n> > libdl,\n> > liblttng,\n> > + libperfetto,\n> > libudev,\n> > libyaml,\n> > ]\n> > diff --git a/src/libcamera/perfetto/meson.build\n> b/src/libcamera/perfetto/meson.build\n> > new file mode 100644\n> > index 00000000..119a1ad7\n> > --- /dev/null\n> > +++ b/src/libcamera/perfetto/meson.build\n> > @@ -0,0 +1,6 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +perfetto_sources = files([\n> > + 'pipeline_perfetto.cpp',\n> > + 'request_perfetto.cpp',\n> > +])\n> > diff --git a/src/libcamera/perfetto/pipeline_perfetto.cpp\n> b/src/libcamera/perfetto/pipeline_perfetto.cpp\n> > new file mode 100644\n> > index 00000000..07b82ffd\n> > --- /dev/null\n> > +++ b/src/libcamera/perfetto/pipeline_perfetto.cpp\n> > @@ -0,0 +1,24 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * pipeline.tp - Tracepoints for pipelines\n> > + */\n> > +\n> > +#include \"libcamera/internal/tracepoints.h\"\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char\n> *func)\n> > +{\n> > + // TODO: Consider TRACE_EVENT_BEGIN\n>\n> From what I've read, I think TRACE_EVENT_BEGIN is better.\n>\n> In the perfetto tracepoints generator we could automatically detect\n> _begin and _end suffixes and convert those to TRACE_EVENT_BEGIN and\n> TRACE_EVENT_END.\n>\n>\nWe might need to add an argument to set the track id, or else Perfetto\nlibrary doesn't know which `begin` the `end` belongs to.\n\n\n> > + TRACE_EVENT(\"libcamera\", \"ipa_call_begin\",\n> > + \"pipeline_name\", pipe,\n> > + \"function_name\", func);\n> > +}\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char\n> *func)\n> > +{\n> > + // TODO: Consider TRACE_EVENT_END\n> > + TRACE_EVENT(\"libcamera\", \"ipa_call_end\",\n> > + \"pipeline_name\", pipe,\n> > + \"function_name\", func);\n> > +}\n> > diff --git a/src/libcamera/perfetto/request_perfetto.cpp\n> b/src/libcamera/perfetto/request_perfetto.cpp\n> > new file mode 100644\n> > index 00000000..2cfff28e\n> > --- /dev/null\n> > +++ b/src/libcamera/perfetto/request_perfetto.cpp\n> > @@ -0,0 +1,73 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2022, Google Inc.\n> > + *\n> > + * request.tp - Tracepoints for the request object\n> > + */\n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/request.h\"\n> > +#include \"libcamera/internal/tracepoints.h\"\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request\",\n> > + \"request_ptr\", reinterpret_cast(req),\n> > + \"cookie\", req->cookie(),\n> > + \"status\", req->status()); // TODO\n>\n> (What is this TODO for?)\n>\n> > +}\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request_construct\",\n> > + \"request_ptr\", reinterpret_cast(req));\n> > +}\n>\n> So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments\n> and fields and how to obtain the fields from the args. We can\n> instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only\n> have to specify the args, and the fields are already defined and come\n> from the class. This reduces duplication in the tracepoint definitions.\n>\n> I'm wondering from what you've implemented here that perfetto doesn't\n> support this? I don't see anything in the perfetto docs that suggest\n> that this is possible either. Although we can mitigate this issue by\n> generating the perfetto tracepoints :) (this is another reason why I\n> think we should use the lttng tracepoints definition as the main one and\n> generate the perfetto ones from those)\n>\n>\nI've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that\nthere are limited features in lttng [1], and if we use the lttng tracepoints\ndefinition as the source of truth, it's hard/weird to implement some\nPerfetto\nconcepts, like nested slices (lifetime of trace events) and flows [2].\n\nIt's not impossible, as you mentioned above, that we could detect the prefix\nand suffix of each libcamera/lttng macro. However, it's a weird code parser\nand generator design.\n\n[1]: https://lttng.org/man/3/lttng-ust/v2.8/\n[2]: https://perfetto.dev/docs/instrumentation/track-events\n\n> +\n> > +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request_destroy\",\n> > + \"request_ptr\", reinterpret_cast(req));\n> > +}\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request_reuse\",\n> > + \"request_ptr\", reinterpret_cast(req));\n> > +}\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request_queue\",\n> > + \"request_ptr\", reinterpret_cast(req));\n> > +}\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request *req)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request_device_queue\",\n> > + \"request_ptr\", reinterpret_cast(req));\n> > +}\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private\n> *req)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request_complete\",\n> > + \"request_private_ptr\",\n> reinterpret_cast(req));\n> > +}\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private\n> *req)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request_cancel\",\n> > + \"request_private_ptr\",\n> reinterpret_cast(req));\n> > +}\n> > +\n> > +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(\n> > + libcamera::Request::Private *req,\n> > + libcamera::FrameBuffer *buf)\n> > +{\n> > + TRACE_EVENT(\"libcamera\", \"request_complete_buffer\",\n> > + \"request_private_ptr\",\n> reinterpret_cast(req),\n> > + \"cookie\", req->_o()->cookie(),\n> > + \"status\", req->_o()->status(), //\n> TODO\n> > + \"buffer_ptr\", reinterpret_cast(buf),\n> > + \"buffer_status\", buf->metadata().status); // TODO\n> > +}\n>\n> Alright, I'm going to use this as an example.\n>\n> First, the tracepoint definition. In lttng we have:\n>\n> TRACEPOINT_EVENT(\n> libcamera,\n> request_complete_buffer,\n> TP_ARGS(\n> libcamera::Request::Private *, req,\n> libcamera::FrameBuffer *, buf\n> ),\n> TP_FIELDS(\n> ctf_integer_hex(uintptr_t, request,\n> reinterpret_cast(req))\n> ctf_integer(uint64_t, cookie,\n> req->_o()->cookie())\n> ctf_integer(int, status,\n> req->_o()->status())\n> ctf_integer_hex(uintptr_t, buffer,\n> reinterpret_cast(buf))\n> ctf_enum(libcamera, buffer_status, uint32_t, buf_status,\n> buf->metadata().status)\n> )\n> )\n>\n> And to raise (idk the right verb) the tracepoint, we use\n> tracepoint(libcamera, request_complete_buffer, request_ptr, buffer_ptr).\n> In perfetto as far as I understand, it's raised using TRACE_EVENT(...)\n> and you have to specify everything that lttng wraps away in TP_FIELDS in\n> the tracepoint definition itself. For this reason, I think it's good to\n> have the intermediary functions.\n>\n> The idea is to generate both the perfetto tracepoint definition and the\n> intermediary function from the lttng definition. I've studied the\n> perfetto tracepoint definition docs a bit more than last time, and I\n> think that going from lttng's definitions to perfetto's is better.\n>\n> I was going to walk through an example, but I'm not sure how because it\n> seems pretty straightforward to me. We can parse the TRACEPOINT_EVENT,\n> and extract the category name and tracepoint name into the function name\n> and TRACE_EVENT parameters. TP_ARGS also can get parsed and copied into\n> the function parameters almost as-is. TP_FIELDS needs a bit of\n> transformation, but it's just extracting the field name and\n> transformation, and then adding an extra cast for the type. ctf_enum\n> would be a bit more involved though, but we have definitions in .tp\n> files for the enums that map the integer values to strings, so we can\n> use that. It might involve generating an intermediate file for all the\n> enums, or I guess we could just generate enum definitions and embed that\n> in the perfetto tracepoint function definitions cpp file.\n>\n> For TRACEPOINT_EVENT_CLASS and TRACEPOINT_EVENT_INSTANCE there's just\n> one extra level of indirection to obtain TP_FIELDS, but otherwise we\n> just match it based on the tracepoint class name.\n>\n> As for \"how\", we can use either ply or a custom-made simple C parser for\n> parsing, and then we'd have to make a custom code generator. If we go\n> homemade C parser route it doesn't have to be too complex, as we only\n> have TRACEPOINT_EVENT, TRACEPOINT_EVENT_CLASS, and\n> TRACEPOINT_EVENT_INSTANCE to parse, and the complex expressions are\n> simply copied. And then for the code generation part we can use jinja2.\n> imo the most difficult part is plumbing it through meson, but we've done\n> that before in the IPC layer, so that has examples of how to do code\n> generation -> compilation via meson [2] [3] [4], as well as examples for\n> jinja2 (although it's actually embedded in mojom).\n>\n>\nI agree that in the current functionalities, it's not that hard to implement\na homemade C/python parser, and generate Perfetto code with jinja2.\nHowever, we not only \"limit\" the functionalities of Perfetto with lttng's\ndefinition, but also add another layer to be maintained.\nIt's true that having duplication of libcamera tracepoint definitions\nrequires more maintenance work when updating the libcamera\ntracepoints, while the effort to maintain the parser and code generation\nis also non-negligible.\n\nI also think that creating a new language from which we generate\ntracepoints for both is an overkill though. It seems that we don't have\na perfect way to support both lttng and Perfetto...\n\nBTW, I tried lex and yacc to parse lttng macros, and tbh it's very painful\nthat it's very hard to use lex to recognize C/C++ type as a token... So\nyeah if we end up deciding to take this route, a homemade parser from\nscratch makes more sense to me.\n\nSo let me list the approaches with pros and cons:\n1. Generate Perfetto code from lttng macros.\n Pros: Easy to update libcamera tracepoint macros.\n Cons: Add maintenance work with the additional parser and code\ngenerator; hard to implement Perfetto features that lttng doesn't have.\n\n2. Creating a new language from which we generate both Perfetto\nand lttng macros.\n Pros: Easy to update libcamera tracepoint macros.\n Cons: Probably an overkill; hard to cover all features from both\nPerfetto and lttng.\n\n3. As this patch implements: have duplicated libcamera tracepoint\ndefinitions.\n Pros: Easy to add libcamera tracepoint macros to cover all features\nfrom Perfetto & lttng.\n Cons: Developer needs to update both sides when the definition of\nlibcamera tracepoints changes; Unused macro definitions might be\nadopted in both sides, if a feature is only available in one trace library.\n\n4. Add another set of libcamera tracepoints for Perfetto, or just use\nPerfetto macros directly in libcamera's source code.\n Pros: Two isolated trace stacks that don't influence each other. All\nfeatures can be implemented easily.\n Cons: Duplicated trace code in source code that might lead to\nconfusion; developers also need to update both macros when a\nnew trace is needed.\n\nI think approach 3rd and 4th are easier to maintain in the long\nterm. WDYT?\n\nAlso, I'd like to know if you think Perfetto will be used in other linux\ndistributions than ChromeOS or Android.\n\nChinglin, please help amend anything I missed as well :)\n\nBR,\nHarvey\n\n[2] utils/ipc/meson.build\n> [3] include/libcamera/ipa/meson.build\n> [4] src/libcamera/ipa/meson.build\n>\n> Would you be able to do this?\n>\n>\n> Paul\n>\n> > diff --git a/src/libcamera/tracepoints.cpp\n> b/src/libcamera/tracepoints.cpp\n> > index 0173b75a..a07ea531 100644\n> > --- a/src/libcamera/tracepoints.cpp\n> > +++ b/src/libcamera/tracepoints.cpp\n> > @@ -4,7 +4,18 @@\n> > *\n> > * tracepoints.cpp - Tracepoints with lttng\n> > */\n> > +\n> > +#if HAVE_PERFETTO\n> > +\n> > +#include \"libcamera/internal/tracepoints.h\"\n> > +\n> > +PERFETTO_TRACK_EVENT_STATIC_STORAGE();\n> > +\n> > +#else\n> > +\n> > #define TRACEPOINT_CREATE_PROBES\n> > #define TRACEPOINT_DEFINE\n> >\n> > #include \"libcamera/internal/tracepoints.h\"\n> > +\n> > +#endif /* HAVE_PERFETTO */\n> > --\n> > 2.39.0.314.g84b9a713c41-goog\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 20A68BDC71\n\tfor ;\n\tTue, 30 Jul 2024 13:30:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E97F063374;\n\tTue, 30 Jul 2024 15:30:54 +0200 (CEST)","from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com\n\t[IPv6:2a00:1450:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E270C61994\n\tfor ;\n\tTue, 30 Jul 2024 15:30:50 +0200 (CEST)","by mail-lj1-x22b.google.com with SMTP id\n\t38308e7fff4ca-2ee920b0781so51413311fa.1\n\tfor ;\n\tTue, 30 Jul 2024 06:30:50 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"XfFmpd4E\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1722346250; x=1722951050;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=oeIa9MMmLm8Ud2FjhPnNM+cnyxf1uUfTEeB6ZR54Gtc=;\n\tb=XfFmpd4ES1xHXLS2gE4doaS6TL5dSs/x//lRReFqAsnpFWvXLiU7FUk9OraJsIfYNS\n\tIqGEFWjzkH6fB+yhwrZfjxNnYoGkyt1MiyYfMaix5IMbbfFQBk9HG3v3ZXbvEbgtRoTb\n\tIvQWPiL2PRy6WD+BGBDUtnfekPG6OJHAtRsco=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1722346250; x=1722951050;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=oeIa9MMmLm8Ud2FjhPnNM+cnyxf1uUfTEeB6ZR54Gtc=;\n\tb=wCQ6OAoSmHHM3fVgeMSD4MojgGfC5Fs6acpJYIxW5mQ7ZGW6RZT9XkPFUmONcYNF/K\n\tffAz+PgYgy+FuJR7KqB1DE/09OnsWW3GbV7H+hR2wkTLNG9W/YQEqTkj5oEkUxOuTWqn\n\t6OpyLMO/daMY0QmtO7uYGBx5CiMWKRaBHq7xFyEw4VUAjAPeMvPwYVsf/Wt9Km+KPY/w\n\tXBQZ1OIdXEiCX/7S8HRJkKcnGe6Exu2epWDNVMtAZxRu5OOikIS+C3s2lhiWdol1WD1/\n\ta/ndRBJu70bk3FbhomKnR3yrI+4OwE01LsG/FsboMJlbfgCoU0BU1KRxUinNnQl+S0wt\n\txf8A==","X-Gm-Message-State":"AOJu0YxkQ5kr7qt/txm83amBjCgc7/ZxZmRKl36UUJBUsJftXGjxnavR\n\ttnkQm8wET4TWiKmcHSi0XnhtwuK1zVeN8rFYYUW8dIUcRvnw6tR3pIoMsP/8Zl2y/ARWC6iorAr\n\tU3dhDqOzPlNi6uAVi3TkTyeh500fxwWSt8F+t","X-Google-Smtp-Source":"AGHT+IE2f1sjhw05ZkBpsOS0dunRztaKIILhsmzUp83jLRVvgMVOIcleGitnJQg75zx8ryaz2UPHTBJmTgzPTkfsso0=","X-Received":"by 2002:a2e:9854:0:b0:2ef:c8a1:ff6 with SMTP id\n\t38308e7fff4ca-2f12edfb4cfmr73059911fa.1.1722346249550;\n\tTue, 30 Jul 2024 06:30:49 -0700 (PDT)","MIME-Version":"1.0","References":"<20221220070523.3212844-1-chenghaoyang@google.com>\n\t<20221220070523.3212844-2-chenghaoyang@google.com>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Tue, 30 Jul 2024 21:30:38 +0800","Message-ID":"","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","To":"Paul Elder ,\n\tChinglin Yu ","Cc":"libcamera-devel@lists.libcamera.org,\n\tHarvey Yang ","Content-Type":"multipart/alternative; boundary=\"000000000000628f6f061e76fc08\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":30886,"web_url":"https://patchwork.libcamera.org/comment/30886/","msgid":"","date":"2024-08-22T15:07:39","subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Gentle ping: perfetto is an important feature in CrOS to do the debugging.\nIt'd be nice to have it supported earlier.\nThanks!\n\nOn Tue, Jul 30, 2024 at 3:30 PM Cheng-Hao Yang \nwrote:\n\n> Hi Paul,\n>\n> Sorry for the late reply. I've been working on mtkisp7's migration :p\n>\n> On Tue, Dec 27, 2022 at 12:09 PM Paul Elder \n> wrote:\n>\n>> Hi Harvey,\n>>\n>> On Tue, Dec 20, 2022 at 07:05:23AM +0000, Harvey Yang via libcamera-devel\n>> wrote:\n>> > As ChromeOS is using perfetto (project named as CrOSetto). When ChromeOS\n>> > uses libcamera, it should use perfetto to collect traces instead of\n>> > lttng.\n>>\n>> I'll start with the elephant in the room: we cannot have duplicate\n>> tracepoint definitions. There must be only one definition of the\n>> tracepoints, and either one or both of perfetto's and lttng's\n>> tracepoints must be generated from that.\n>>\n>> For instance, I've already got a series pending [1] that expands on the\n>> request tracepoints quite a bit; it's clearly unscalable to deal with\n>> two duplicate sets of tracepoints (I'm not sure why I have to defend\n>> deduplication, but anyway just trying to be clear about my stance).\n>>\n>> [1] https://patchwork.libcamera.org/project/libcamera/list/?series=3668\n>>\n>> More on this in the relevant section in the patch.\n>>\n>> >\n>> > Signed-off-by: Harvey Yang \n>> > ---\n>> > Documentation/guides/tracing.rst | 125 ++++++++++++++----\n>> > include/libcamera/internal/tracepoints.h.in | 38 +++++-\n>> > .../internal/tracepoints/meson.build | 25 ++--\n>> > .../internal/tracepoints/pipeline.perfetto | 10 ++\n>> > .../internal/tracepoints/request.perfetto | 30 +++++\n>> > meson.build | 7 +-\n>> > src/android/cros/camera3_hal.cpp | 5 +\n>> > src/android/cros/meson.build | 1 +\n>> > src/libcamera/meson.build | 14 +-\n>> > src/libcamera/perfetto/meson.build | 6 +\n>> > src/libcamera/perfetto/pipeline_perfetto.cpp | 24 ++++\n>> > src/libcamera/perfetto/request_perfetto.cpp | 73 ++++++++++\n>> > src/libcamera/tracepoints.cpp | 11 ++\n>> > 13 files changed, 325 insertions(+), 44 deletions(-)\n>> > create mode 100644\n>> include/libcamera/internal/tracepoints/pipeline.perfetto\n>> > create mode 100644\n>> include/libcamera/internal/tracepoints/request.perfetto\n>> > create mode 100644 src/libcamera/perfetto/meson.build\n>> > create mode 100644 src/libcamera/perfetto/pipeline_perfetto.cpp\n>> > create mode 100644 src/libcamera/perfetto/request_perfetto.cpp\n>> >\n>> > diff --git a/Documentation/guides/tracing.rst\n>> b/Documentation/guides/tracing.rst\n>> > index ae960d85..bd5b3453 100644\n>> > --- a/Documentation/guides/tracing.rst\n>> > +++ b/Documentation/guides/tracing.rst\n>> > @@ -16,33 +16,50 @@ at periodic points in time. This can be done with\n>> other tools such as\n>> > callgrind, perf, gprof, etc., without modification to the application,\n>> > and is out of scope for this guide.\n>> >\n>> > +Library: Perfetto vs lttng-ust\n>> > +------------------------------\n>> > +\n>> > +To integrate with CrOS tracing infrastructure, which uses perfetto (or\n>> called\n>>\n>> The first instance of \"CrOS\" should probably be spelled out.\n>>\n>> > +CrOSetto in CrOS), we implement the tracepoint macros (will be\n>> described below)\n>> > +with two different libraries: CrOS with perfetto, and the rest with\n>> lttng-ust.\n>> > +\n>> > Compiling\n>> > ---------\n>> >\n>> > -To compile libcamera with tracing support, it must be enabled through\n>> the\n>> > -meson ``tracing`` option. It depends on the lttng-ust library\n>> (available in the\n>> > -``liblttng-ust-dev`` package for Debian-based distributions).\n>> > -By default the tracing option in meson is set to ``auto``, so if\n>> > -liblttng is detected, it will be enabled by default. Conversely, if\n>> the option\n>> > -is set to disabled, then libcamera will be compiled without tracing\n>> support.\n>> > +To compile libcamera with tracing support in CrOS, it must be enabled\n>> through\n>>\n>> s/ in CrOS//\n>>\n>> > +the meson ``tracing`` option. In CrOS, it depends on the perfetto\n>> library,\n>> > +which is by default available in CrOS; In the rest, it depends on the\n>> lttng-ust\n>>\n>> s/;/./\n>>\n>> s/the rest/non-CrOS/\n>>\n>> > +library (available in the ``liblttng-ust-dev`` package for Debian-based\n>> > +distributions).\n>> > +By default the tracing option in meson is set to ``auto``, so if the\n>> library is\n>> > +detected, it will be enabled by default. Conversely, if the option is\n>> set to\n>> > +disabled, then libcamera will be compiled without tracing support.\n>> >\n>> > Defining tracepoints\n>> > --------------------\n>> >\n>> > libcamera already contains a set of tracepoints. To define additional\n>> > -tracepoints, create a file\n>> > -``include/libcamera/internal/tracepoints/{file}.tp``, where ``file``\n>> is a\n>> > -reasonable name related to the category of tracepoints that you wish to\n>> > -define. For example, the tracepoints file for the Request object is\n>> called\n>> > -``request.tp``. An entry for this file must be added in\n>> > -``include/libcamera/internal/tracepoints/meson.build``.\n>> > -\n>> > -In this tracepoints file, define your tracepoints `as mandated by lttng\n>> > +tracepoints, create two files ``{file}.tp`` and ``{file}.perfetto``,\n>> for lttng\n>> > +and perfetto respectively, under\n>> ``include/libcamera/internal/tracepoints/``,\n>> > +and the perfetto implementation file\n>> ``src/libcamera/{file}_perfetto.cpp``,\n>> > +where ``file`` is a reasonable name related to the category of\n>> tracepoints that\n>> > +you wish to define. For example, the tracepoints files for the Request\n>> object is\n>> > +called ``request.tp`` and ``request.perfetto``. Entries for the files\n>> must be\n>> > +added in ``include/libcamera/internal/tracepoints/meson.build``.\n>> > +\n>> > +In the perfetto tracepoints files, declare the tracepoint functions in\n>> > +``{file}.perfetto``, and define them in ``{file}_perfetto.cpp``, `as\n>> mandated\n>> > +by perfetto > >`_.\n>> > +Currently the only enabled perfetto::Category is ``libcamera``. If you\n>> intend to\n>> > +use another category, remember to define it in\n>> > +``include/libcamera/internal/tracepoints.h.in``, and enable it when\n>> collecting\n>> > +traces.\n>>\n>> These sections will have to be fixed.\n>>\n>> > +\n>> > +In the lttng tracepoints file, define your tracepoints `as mandated by\n>> lttng\n>> > `_. The header boilerplate must\n>> *not* be\n>> > included (as it will conflict with the rest of our infrastructure), and\n>> > only the tracepoint definitions (with the ``TRACEPOINT_*`` macros)\n>> should be\n>> > included.\n>> > -\n>> > All tracepoint providers shall be ``libcamera``. According to lttng,\n>> the\n>> > tracepoint provider should be per-project; this is the rationale for\n>> this\n>> > decision. To group tracepoint events, we recommend using\n>> > @@ -68,9 +85,9 @@ Then to use the tracepoint:\n>> >\n>> > ``LIBCAMERA_TRACEPOINT({tracepoint_event}, args...)``\n>> >\n>> > -This macro must be used, as opposed to lttng's macros directly, because\n>> > -lttng is an optional dependency of libcamera, so the code must compile\n>> and run\n>> > -even when lttng is not present or when tracing is disabled.\n>> > +This macro must be used, as opposed to perfetto's or lttng's macros\n>> directly,\n>> > +because tracing support is optional of libcamera, so the code must\n>> compile and\n>>\n>> s/of/in/\n>>\n>> > +run even when perfetto or lttng are not present or when tracing is\n>> disabled.\n>> >\n>> > The tracepoint provider name, as declared in the tracepoint\n>> definition, is not\n>> > included in the parameters of the tracepoint.\n>> > @@ -87,21 +104,75 @@ respectively. These are the tracepoints that our\n>> sample analysis script\n>> > (see \"Analyzing a trace\") scans for when computing statistics on IPA\n>> call time.\n>> >\n>> > Using tracepoints (from an application)\n>> > ----------------------------------------\n>> > +---------------------------------------------\n>> >\n>> > As applications are not part of libcamera, but rather users of\n>> libcamera,\n>> > applications should seek their own tracing mechanisms. For ease of\n>> tracing\n>> > the application alongside tracing libcamera, it is recommended to also\n>> > -`use lttng <\n>> https://lttng.org/docs/#doc-tracing-your-own-user-application>`_.\n>> > +`use lttng <\n>> https://lttng.org/docs/#doc-tracing-your-own-user-application>`_,\n>> > +or `use perfetto <\n>> https://perfetto.dev/docs/instrumentation/tracing-sdk>`_.\n>> >\n>> > Using tracepoints (from closed-source IPA)\n>> > -------------------------------------------\n>> > +------------------------------------------------\n>>\n>> Underline length.\n>>\n>> > +\n>> > +Similar to applications, closed-source IPAs can simply use perfetto or\n>> lttng\n>> > +on their own, or any other tracing mechanism if desired.\n>> > +\n>> > +Collecting a perfetto trace\n>> > +---------------------------\n>> > +\n>> > +A trace can be collected with the following steps:\n>> > +\n>> > +1. Start `traced` if it hasn't been started.\n>> > +.. code-block:: bash\n>> > +\n>> > + start traced\n>> > +\n>> > +2. Start a consumer that includes “track_event” data source in the\n>> trace\n>> > + config, and “libcamera” category.\n>> > +.. code-block:: bash\n>> > +\n>> > + perfetto -c - --txt -o /tmp/perfetto-trace \\\n>>\n>> (Reply to your comment in the other email) I think this is sufficient,\n>> as apparently the option for a config file is documented in the help\n>> text.\n>>\n>> > + <> > +\n>> > + buffers: {\n>> > + size_kb: 63488\n>> > + fill_policy: DISCARD\n>> > + }\n>> > + buffers: {\n>> > + size_kb: 2048\n>> > + fill_policy: DISCARD\n>> > + }\n>> > + data_sources: {\n>> > + config {\n>> > + name: \"track_event\"\n>> > + track_event_config {\n>> > + enabled_categories: \"libcamera\"\n>> > + }\n>> > + }\n>> > + }\n>> > + duration_ms: 10000\n>> > +\n>> > + EOF\n>> > +\n>> > +3. Execute the libcamera behavior you intend to trace during the set\n>> duration\n>> > + (10000 ms) in the above example.\n>> > +\n>> > +4. After the consumer (cmd `perfetto`) is done, you can find the trace\n>> result\n>> > + in ``/tmp/perfetto-trace``.\n>> > +\n>> > +Analyzing a perfetto trace\n>> > +--------------------------\n>> > +\n>> > +Follow the `guide `_\n>> to\n>> > +visualize the trace.\n>> >\n>> > -Similar to applications, closed-source IPAs can simply use lttng on\n>> their own,\n>> > -or any other tracing mechanism if desired.\n>> > +In other words, upload the trace to `Perfetto UI <\n>> https://ui.perfetto.dev/>`_,\n>> > +where you can check the timeline of tracepoints on each\n>> process/thread. You can\n>> > +also run SQL queries to do analysis.\n>> >\n>> > -Collecting a trace\n>> > -------------------\n>> > +Collecting an lttng trace\n>> > +------------------------\n>>\n>> Underline length.\n>>\n>> >\n>> > A trace can be collected fairly simply from lttng:\n>> >\n>> > @@ -123,8 +194,8 @@ viewed by: ``lttng view -t $PATH_TO_TRACE``, where\n>> ``$PATH_TO_TRACE`` is the\n>> > path that was printed when the session was created. This is the same\n>> path that\n>> > is used when analyzing traces programatically, as described in the\n>> next section.\n>> >\n>> > -Analyzing a trace\n>> > ------------------\n>> > +Analyzing a lttng trace\n>>\n>> s/a/an/\n>>\n>> > +-----------------------\n>> >\n>> > As mentioned above, while an lttng tracing session exists and the\n>> trace is not\n>> > running, the trace output can be viewed as text by ``lttng view``.\n>> > diff --git a/include/libcamera/internal/tracepoints.h.in\n>> b/include/libcamera/internal/tracepoints.h.in\n>> > index d0fc1365..b6c4abea 100644\n>> > --- a/include/libcamera/internal/tracepoints.h.in\n>> > +++ b/include/libcamera/internal/tracepoints.h.in\n>> > @@ -9,7 +9,24 @@\n>> > #ifndef __LIBCAMERA_INTERNAL_TRACEPOINTS_H__\n>> > #define __LIBCAMERA_INTERNAL_TRACEPOINTS_H__\n>> >\n>> > -#if HAVE_TRACING\n>> > +#if HAVE_PERFETTO\n>> > +\n>> > +#include \n>> > +\n>> > +PERFETTO_DEFINE_CATEGORIES(\n>> > + perfetto::Category(\"libcamera\")\n>> > + .SetDescription(\"Events from libcamera\"));\n>> > +\n>> > +#define LIBCAMERA_TRACEPOINT(t_name, ...) \\\n>> > +LIBCAMERA_TRACE_EVENT_##t_name(__VA_ARGS__)\n>> > +\n>> > +#define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \\\n>> > +LIBCAMERA_TRACE_EVENT_ipa_call_begin(#pipe, #func)\n>> > +\n>> > +#define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \\\n>> > +LIBCAMERA_TRACE_EVENT_ipa_call_end(#pipe, #func)\n>> > +\n>> > +#elif HAVE_LTTNG /* !HAVE_PERFETTO */\n>> > #define LIBCAMERA_TRACEPOINT(...) tracepoint(libcamera, __VA_ARGS__)\n>> >\n>> > #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func) \\\n>> > @@ -18,7 +35,7 @@ tracepoint(libcamera, ipa_call_begin, #pipe, #func)\n>> > #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func) \\\n>> > tracepoint(libcamera, ipa_call_end, #pipe, #func)\n>> >\n>> > -#else\n>> > +#else /* !HAVE_PERFETTO && !HAVE_LTTNG */\n>> >\n>> > namespace {\n>> >\n>> > @@ -34,12 +51,17 @@ inline void unused([[maybe_unused]] Args&& ...args)\n>> > #define LIBCAMERA_TRACEPOINT_IPA_BEGIN(pipe, func)\n>> > #define LIBCAMERA_TRACEPOINT_IPA_END(pipe, func)\n>> >\n>> > -#endif /* HAVE_TRACING */\n>> > +#endif /* HAVE_PERFETTO */\n>> >\n>> > #endif /* __LIBCAMERA_INTERNAL_TRACEPOINTS_H__ */\n>> >\n>> > +#if HAVE_PERFETTO || HAVE_LTTNG\n>> > +\n>> > +#if HAVE_PERFETTO\n>> >\n>> > -#if HAVE_TRACING\n>> > +#include \n>> > +\n>> > +#else /* HAVE_LTTNG */\n>> >\n>> > #undef TRACEPOINT_PROVIDER\n>> > #define TRACEPOINT_PROVIDER libcamera\n>> > @@ -47,15 +69,21 @@ inline void unused([[maybe_unused]] Args&& ...args)\n>> > #undef TRACEPOINT_INCLUDE\n>> > #define TRACEPOINT_INCLUDE \"{{path}}\"\n>> >\n>> > +#endif /* HAVE_PERFETTO */\n>> > +\n>> > #if !defined(INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H) ||\n>> defined(TRACEPOINT_HEADER_MULTI_READ)\n>> > #define INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H\n>>\n>> I'm pretty sure #endif /* HAVE_PERFETTO */ should come here (unless\n>> perfetto also needs this? Does it?).\n>>\n>> >\n>> > +#if HAVE_LTTNG\n>> > #include \n>> > +#endif /* HAVE_LTTNG */\n>> >\n>> > {{source}}\n>> >\n>> > #endif /* INCLUDE_LIBCAMERA_INTERNAL_TRACEPOINTS_TP_H */\n>>\n>> Same for this.\n>>\n>> I wonder if it's easier just to have a top-level `if HAVE_LTTNG and if\n>> HAVE_PERFETTO` and just duplicate {{source}}. It's only one line that's\n>> duplicated anyway.\n>>\n>> >\n>> > +#if HAVE_LTTNG\n>> > #include \n>> > +#endif /* HAVE_LTTNG */\n>> >\n>> > -#endif /* HAVE_TRACING */\n>> > +#endif /* HAVE_PERFETTO || HAVE_LTTNG */\n>> > diff --git a/include/libcamera/internal/tracepoints/meson.build\n>> b/include/libcamera/internal/tracepoints/meson.build\n>> > index d9b2fca5..ff5aece6 100644\n>> > --- a/include/libcamera/internal/tracepoints/meson.build\n>> > +++ b/include/libcamera/internal/tracepoints/meson.build\n>> > @@ -1,12 +1,19 @@\n>> > # SPDX-License-Identifier: CC0-1.0\n>> >\n>> > -# enum files must go first\n>> > -tracepoint_files = files([\n>> > - 'buffer_enums.tp',\n>> > - 'request_enums.tp',\n>> > -])\n>> > +if get_option('android').enabled() and get_option('android_platform')\n>> == 'cros'\n>> > + tracepoint_files = files([\n>> > + 'pipeline.perfetto',\n>> > + 'request.perfetto',\n>> > + ])\n>> > +else\n>> > + # enum files must go first\n>> > + tracepoint_files = files([\n>> > + 'buffer_enums.tp',\n>> > + 'request_enums.tp',\n>> > + ])\n>> >\n>> > -tracepoint_files += files([\n>> > - 'pipeline.tp',\n>> > - 'request.tp',\n>> > -])\n>> > + tracepoint_files += files([\n>> > + 'pipeline.tp',\n>> > + 'request.tp',\n>> > + ])\n>> > +endif\n>>\n>> And this is going to need a bit more moving parts for code generation...\n>>\n>> > diff --git a/include/libcamera/internal/tracepoints/pipeline.perfetto\n>> b/include/libcamera/internal/tracepoints/pipeline.perfetto\n>> > new file mode 100644\n>> > index 00000000..5f45295e\n>> > --- /dev/null\n>> > +++ b/include/libcamera/internal/tracepoints/pipeline.perfetto\n>> > @@ -0,0 +1,10 @@\n>> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> > +/*\n>> > + * Copyright (C) 2022, Google Inc.\n>> > + *\n>> > + * pipeline.tp - Tracepoints for pipelines\n>> > + */\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char\n>> *func);\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char\n>> *func);\n>>\n>> Basically, we already have definitions for these tracepoints in\n>> include/libcamera/internal/tracepoints/pipeline.tp. It's obviously not a\n>> good idea to have duplicate definitions.\n>>\n>> So we either need to use lttng's tracepoints definitions to generate\n>> perfetto's here, or we create a new language from which we generate\n>> tracepoints for both. imo the latter is overkill (and costs a lot more\n>> effort) so probably the former is better.\n>>\n>> I'll use the request tracepoints as examples below just because they\n>> have a bit more beef than these pipeline tracepoints which are just two\n>> strings.\n>>\n>> > diff --git a/include/libcamera/internal/tracepoints/request.perfetto\n>> b/include/libcamera/internal/tracepoints/request.perfetto\n>> > new file mode 100644\n>> > index 00000000..fd6a42a4\n>> > --- /dev/null\n>> > +++ b/include/libcamera/internal/tracepoints/request.perfetto\n>> > @@ -0,0 +1,30 @@\n>> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> > +/*\n>> > + * Copyright (C) 2022, Google Inc.\n>> > + *\n>> > + * request.tp - Tracepoints for the request object\n>> > + */\n>> > +\n>> > +#include \n>> > +\n>> > +#include \n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req);\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req);\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req);\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req);\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req);\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request\n>> *req);\n>> > +\n>> > +void\n>> LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private *req);\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private\n>> *req);\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(\n>> > + libcamera::Request::Private *req,\n>> > + libcamera::FrameBuffer * buf);\n>> > diff --git a/meson.build b/meson.build\n>> > index e4c0be16..79514b5b 100644\n>> > --- a/meson.build\n>> > +++ b/meson.build\n>> > @@ -156,7 +156,11 @@ libcamera_includes = include_directories('include')\n>> > py_modules = []\n>> >\n>> > # Libraries used by multiple components\n>> > -liblttng = dependency('lttng-ust', required : get_option('tracing'))\n>> > +libperfetto = dependency('perfetto', required :\n>> get_option('tracing').enabled()\n>> > + and get_option('android').enabled()\n>> > + and get_option('android_platform') == 'cros')\n>> > +liblttng = cc.find_library('lttng-ust', required :\n>> get_option('tracing').enabled()\n>> > + and not libperfetto.found())\n>> >\n>> > # Pipeline handlers\n>> > #\n>> > @@ -208,6 +212,7 @@ py_mod.find_installation('python3', modules:\n>> py_modules)\n>> > summary({\n>> > 'Enabled pipelines': pipelines,\n>> > 'Enabled IPA modules': enabled_ipa_modules,\n>> > + 'Perfetto support': perfetto_enabled,\n>> > 'Tracing support': tracing_enabled,\n>> > 'Android support': android_enabled,\n>> > 'GStreamer support': gst_enabled,\n>> > diff --git a/src/android/cros/camera3_hal.cpp\n>> b/src/android/cros/camera3_hal.cpp\n>> > index fb863b5f..2fbd7f14 100644\n>> > --- a/src/android/cros/camera3_hal.cpp\n>> > +++ b/src/android/cros/camera3_hal.cpp\n>> > @@ -7,10 +7,15 @@\n>> >\n>> > #include \n>> >\n>> > +#include \"libcamera/internal/tracepoints.h\"\n>> > #include \"../camera_hal_manager.h\"\n>> >\n>> > static void set_up([[maybe_unused]]\n>> cros::CameraMojoChannelManagerToken *token)\n>> > {\n>> > + perfetto::TracingInitArgs args;\n>> > + args.backends |= perfetto::kSystemBackend;\n>> > + perfetto::Tracing::Initialize(args);\n>> > + perfetto::TrackEvent::Register();\n>> > }\n>> >\n>> > static void tear_down()\n>> > diff --git a/src/android/cros/meson.build b/src/android/cros/meson.build\n>> > index 35995dd8..68f2bd9e 100644\n>> > --- a/src/android/cros/meson.build\n>> > +++ b/src/android/cros/meson.build\n>> > @@ -9,5 +9,6 @@ android_hal_sources += files([\n>> > ])\n>> >\n>> > android_deps += dependency('libcros_camera')\n>> > +android_deps += dependency('perfetto')\n>> >\n>> > android_cpp_args += ['-DOS_CHROMEOS']\n>> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> > index 0494e808..ba8bf26e 100644\n>> > --- a/src/libcamera/meson.build\n>> > +++ b/src/libcamera/meson.build\n>> > @@ -62,6 +62,7 @@ libthreads = dependency('threads')\n>> >\n>> > subdir('base')\n>> > subdir('ipa')\n>> > +subdir('perfetto')\n>> > subdir('pipeline')\n>> > subdir('proxy')\n>> >\n>> > @@ -89,11 +90,19 @@ if not libcrypto.found()\n>> > warning('Neither gnutls nor libcrypto found, all IPA modules will\n>> be isolated')\n>> > endif\n>> >\n>> > -if liblttng.found()\n>> > +if libperfetto.found()\n>> > + perfetto_enabled = true\n>> > + tracing_enabled = false\n>> > + config_h.set('HAVE_PERFETTO', 1)\n>> > + libcamera_sources += perfetto_sources\n>> > + libcamera_sources += files(['tracepoints.cpp'])\n>> > +elif liblttng.found()\n>> > + perfetto_enabled = false\n>> > tracing_enabled = true\n>> > - config_h.set('HAVE_TRACING', 1)\n>> > + config_h.set('HAVE_LTTNG', 1)\n>> > libcamera_sources += files(['tracepoints.cpp'])\n>> > else\n>> > + perfetto_enabled = false\n>> > tracing_enabled = false\n>> > endif\n>> >\n>> > @@ -154,6 +163,7 @@ libcamera_deps = [\n>> > libcrypto,\n>> > libdl,\n>> > liblttng,\n>> > + libperfetto,\n>> > libudev,\n>> > libyaml,\n>> > ]\n>> > diff --git a/src/libcamera/perfetto/meson.build\n>> b/src/libcamera/perfetto/meson.build\n>> > new file mode 100644\n>> > index 00000000..119a1ad7\n>> > --- /dev/null\n>> > +++ b/src/libcamera/perfetto/meson.build\n>> > @@ -0,0 +1,6 @@\n>> > +# SPDX-License-Identifier: CC0-1.0\n>> > +\n>> > +perfetto_sources = files([\n>> > + 'pipeline_perfetto.cpp',\n>> > + 'request_perfetto.cpp',\n>> > +])\n>> > diff --git a/src/libcamera/perfetto/pipeline_perfetto.cpp\n>> b/src/libcamera/perfetto/pipeline_perfetto.cpp\n>> > new file mode 100644\n>> > index 00000000..07b82ffd\n>> > --- /dev/null\n>> > +++ b/src/libcamera/perfetto/pipeline_perfetto.cpp\n>> > @@ -0,0 +1,24 @@\n>> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> > +/*\n>> > + * Copyright (C) 2022, Google Inc.\n>> > + *\n>> > + * pipeline.tp - Tracepoints for pipelines\n>> > + */\n>> > +\n>> > +#include \"libcamera/internal/tracepoints.h\"\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_ipa_call_begin(const char *pipe, const char\n>> *func)\n>> > +{\n>> > + // TODO: Consider TRACE_EVENT_BEGIN\n>>\n>> From what I've read, I think TRACE_EVENT_BEGIN is better.\n>>\n>> In the perfetto tracepoints generator we could automatically detect\n>> _begin and _end suffixes and convert those to TRACE_EVENT_BEGIN and\n>> TRACE_EVENT_END.\n>>\n>>\n> We might need to add an argument to set the track id, or else Perfetto\n> library doesn't know which `begin` the `end` belongs to.\n>\n>\n>> > + TRACE_EVENT(\"libcamera\", \"ipa_call_begin\",\n>> > + \"pipeline_name\", pipe,\n>> > + \"function_name\", func);\n>> > +}\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_ipa_call_end(const char *pipe, const char\n>> *func)\n>> > +{\n>> > + // TODO: Consider TRACE_EVENT_END\n>> > + TRACE_EVENT(\"libcamera\", \"ipa_call_end\",\n>> > + \"pipeline_name\", pipe,\n>> > + \"function_name\", func);\n>> > +}\n>> > diff --git a/src/libcamera/perfetto/request_perfetto.cpp\n>> b/src/libcamera/perfetto/request_perfetto.cpp\n>> > new file mode 100644\n>> > index 00000000..2cfff28e\n>> > --- /dev/null\n>> > +++ b/src/libcamera/perfetto/request_perfetto.cpp\n>> > @@ -0,0 +1,73 @@\n>> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> > +/*\n>> > + * Copyright (C) 2022, Google Inc.\n>> > + *\n>> > + * request.tp - Tracepoints for the request object\n>> > + */\n>> > +\n>> > +#include \n>> > +\n>> > +#include \"libcamera/internal/request.h\"\n>> > +#include \"libcamera/internal/tracepoints.h\"\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request(libcamera::Request *req)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request\",\n>> > + \"request_ptr\", reinterpret_cast(req),\n>> > + \"cookie\", req->cookie(),\n>> > + \"status\", req->status()); // TODO\n>>\n>> (What is this TODO for?)\n>>\n>> > +}\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_construct(libcamera::Request *req)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request_construct\",\n>> > + \"request_ptr\", reinterpret_cast(req));\n>> > +}\n>>\n>> So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments\n>> and fields and how to obtain the fields from the args. We can\n>> instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only\n>> have to specify the args, and the fields are already defined and come\n>> from the class. This reduces duplication in the tracepoint definitions.\n>>\n>> I'm wondering from what you've implemented here that perfetto doesn't\n>> support this? I don't see anything in the perfetto docs that suggest\n>> that this is possible either. Although we can mitigate this issue by\n>> generating the perfetto tracepoints :) (this is another reason why I\n>> think we should use the lttng tracepoints definition as the main one and\n>> generate the perfetto ones from those)\n>>\n>>\n> I've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that\n> there are limited features in lttng [1], and if we use the lttng\n> tracepoints\n> definition as the source of truth, it's hard/weird to implement some\n> Perfetto\n> concepts, like nested slices (lifetime of trace events) and flows [2].\n>\n> It's not impossible, as you mentioned above, that we could detect the\n> prefix\n> and suffix of each libcamera/lttng macro. However, it's a weird code parser\n> and generator design.\n>\n> [1]: https://lttng.org/man/3/lttng-ust/v2.8/\n> [2]: https://perfetto.dev/docs/instrumentation/track-events\n>\n> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_destroy(libcamera::Request *req)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request_destroy\",\n>> > + \"request_ptr\", reinterpret_cast(req));\n>> > +}\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_reuse(libcamera::Request *req)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request_reuse\",\n>> > + \"request_ptr\", reinterpret_cast(req));\n>> > +}\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_queue(libcamera::Request *req)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request_queue\",\n>> > + \"request_ptr\", reinterpret_cast(req));\n>> > +}\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_device_queue(libcamera::Request\n>> *req)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request_device_queue\",\n>> > + \"request_ptr\", reinterpret_cast(req));\n>> > +}\n>> > +\n>> > +void\n>> LIBCAMERA_TRACE_EVENT_request_complete(libcamera::Request::Private *req)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request_complete\",\n>> > + \"request_private_ptr\",\n>> reinterpret_cast(req));\n>> > +}\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_cancel(libcamera::Request::Private\n>> *req)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request_cancel\",\n>> > + \"request_private_ptr\",\n>> reinterpret_cast(req));\n>> > +}\n>> > +\n>> > +void LIBCAMERA_TRACE_EVENT_request_complete_buffer(\n>> > + libcamera::Request::Private *req,\n>> > + libcamera::FrameBuffer *buf)\n>> > +{\n>> > + TRACE_EVENT(\"libcamera\", \"request_complete_buffer\",\n>> > + \"request_private_ptr\",\n>> reinterpret_cast(req),\n>> > + \"cookie\", req->_o()->cookie(),\n>> > + \"status\", req->_o()->status(), //\n>> TODO\n>> > + \"buffer_ptr\", reinterpret_cast(buf),\n>> > + \"buffer_status\", buf->metadata().status); // TODO\n>> > +}\n>>\n>> Alright, I'm going to use this as an example.\n>>\n>> First, the tracepoint definition. In lttng we have:\n>>\n>> TRACEPOINT_EVENT(\n>> libcamera,\n>> request_complete_buffer,\n>> TP_ARGS(\n>> libcamera::Request::Private *, req,\n>> libcamera::FrameBuffer *, buf\n>> ),\n>> TP_FIELDS(\n>> ctf_integer_hex(uintptr_t, request,\n>> reinterpret_cast(req))\n>> ctf_integer(uint64_t, cookie,\n>> req->_o()->cookie())\n>> ctf_integer(int, status,\n>> req->_o()->status())\n>> ctf_integer_hex(uintptr_t, buffer,\n>> reinterpret_cast(buf))\n>> ctf_enum(libcamera, buffer_status, uint32_t, buf_status,\n>> buf->metadata().status)\n>> )\n>> )\n>>\n>> And to raise (idk the right verb) the tracepoint, we use\n>> tracepoint(libcamera, request_complete_buffer, request_ptr, buffer_ptr).\n>> In perfetto as far as I understand, it's raised using TRACE_EVENT(...)\n>> and you have to specify everything that lttng wraps away in TP_FIELDS in\n>> the tracepoint definition itself. For this reason, I think it's good to\n>> have the intermediary functions.\n>>\n>> The idea is to generate both the perfetto tracepoint definition and the\n>> intermediary function from the lttng definition. I've studied the\n>> perfetto tracepoint definition docs a bit more than last time, and I\n>> think that going from lttng's definitions to perfetto's is better.\n>>\n>> I was going to walk through an example, but I'm not sure how because it\n>> seems pretty straightforward to me. We can parse the TRACEPOINT_EVENT,\n>> and extract the category name and tracepoint name into the function name\n>> and TRACE_EVENT parameters. TP_ARGS also can get parsed and copied into\n>> the function parameters almost as-is. TP_FIELDS needs a bit of\n>> transformation, but it's just extracting the field name and\n>> transformation, and then adding an extra cast for the type. ctf_enum\n>> would be a bit more involved though, but we have definitions in .tp\n>> files for the enums that map the integer values to strings, so we can\n>> use that. It might involve generating an intermediate file for all the\n>> enums, or I guess we could just generate enum definitions and embed that\n>> in the perfetto tracepoint function definitions cpp file.\n>>\n>> For TRACEPOINT_EVENT_CLASS and TRACEPOINT_EVENT_INSTANCE there's just\n>> one extra level of indirection to obtain TP_FIELDS, but otherwise we\n>> just match it based on the tracepoint class name.\n>>\n>> As for \"how\", we can use either ply or a custom-made simple C parser for\n>> parsing, and then we'd have to make a custom code generator. If we go\n>> homemade C parser route it doesn't have to be too complex, as we only\n>> have TRACEPOINT_EVENT, TRACEPOINT_EVENT_CLASS, and\n>> TRACEPOINT_EVENT_INSTANCE to parse, and the complex expressions are\n>> simply copied. And then for the code generation part we can use jinja2.\n>> imo the most difficult part is plumbing it through meson, but we've done\n>> that before in the IPC layer, so that has examples of how to do code\n>> generation -> compilation via meson [2] [3] [4], as well as examples for\n>> jinja2 (although it's actually embedded in mojom).\n>>\n>>\n> I agree that in the current functionalities, it's not that hard to\n> implement\n> a homemade C/python parser, and generate Perfetto code with jinja2.\n> However, we not only \"limit\" the functionalities of Perfetto with lttng's\n> definition, but also add another layer to be maintained.\n> It's true that having duplication of libcamera tracepoint definitions\n> requires more maintenance work when updating the libcamera\n> tracepoints, while the effort to maintain the parser and code generation\n> is also non-negligible.\n>\n> I also think that creating a new language from which we generate\n> tracepoints for both is an overkill though. It seems that we don't have\n> a perfect way to support both lttng and Perfetto...\n>\n> BTW, I tried lex and yacc to parse lttng macros, and tbh it's very painful\n> that it's very hard to use lex to recognize C/C++ type as a token... So\n> yeah if we end up deciding to take this route, a homemade parser from\n> scratch makes more sense to me.\n>\n> So let me list the approaches with pros and cons:\n> 1. Generate Perfetto code from lttng macros.\n> Pros: Easy to update libcamera tracepoint macros.\n> Cons: Add maintenance work with the additional parser and code\n> generator; hard to implement Perfetto features that lttng doesn't have.\n>\n> 2. Creating a new language from which we generate both Perfetto\n> and lttng macros.\n> Pros: Easy to update libcamera tracepoint macros.\n> Cons: Probably an overkill; hard to cover all features from both\n> Perfetto and lttng.\n>\n> 3. As this patch implements: have duplicated libcamera tracepoint\n> definitions.\n> Pros: Easy to add libcamera tracepoint macros to cover all features\n> from Perfetto & lttng.\n> Cons: Developer needs to update both sides when the definition of\n> libcamera tracepoints changes; Unused macro definitions might be\n> adopted in both sides, if a feature is only available in one trace library.\n>\n> 4. Add another set of libcamera tracepoints for Perfetto, or just use\n> Perfetto macros directly in libcamera's source code.\n> Pros: Two isolated trace stacks that don't influence each other. All\n> features can be implemented easily.\n> Cons: Duplicated trace code in source code that might lead to\n> confusion; developers also need to update both macros when a\n> new trace is needed.\n>\n> I think approach 3rd and 4th are easier to maintain in the long\n> term. WDYT?\n>\n> Also, I'd like to know if you think Perfetto will be used in other linux\n> distributions than ChromeOS or Android.\n>\n> Chinglin, please help amend anything I missed as well :)\n>\n> BR,\n> Harvey\n>\n> [2] utils/ipc/meson.build\n>> [3] include/libcamera/ipa/meson.build\n>> [4] src/libcamera/ipa/meson.build\n>>\n>> Would you be able to do this?\n>>\n>>\n>> Paul\n>>\n>> > diff --git a/src/libcamera/tracepoints.cpp\n>> b/src/libcamera/tracepoints.cpp\n>> > index 0173b75a..a07ea531 100644\n>> > --- a/src/libcamera/tracepoints.cpp\n>> > +++ b/src/libcamera/tracepoints.cpp\n>> > @@ -4,7 +4,18 @@\n>> > *\n>> > * tracepoints.cpp - Tracepoints with lttng\n>> > */\n>> > +\n>> > +#if HAVE_PERFETTO\n>> > +\n>> > +#include \"libcamera/internal/tracepoints.h\"\n>> > +\n>> > +PERFETTO_TRACK_EVENT_STATIC_STORAGE();\n>> > +\n>> > +#else\n>> > +\n>> > #define TRACEPOINT_CREATE_PROBES\n>> > #define TRACEPOINT_DEFINE\n>> >\n>> > #include \"libcamera/internal/tracepoints.h\"\n>> > +\n>> > +#endif /* HAVE_PERFETTO */\n>> > --\n>> > 2.39.0.314.g84b9a713c41-goog\n>> >\n>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 24AFBBDE17\n\tfor ;\n\tThu, 22 Aug 2024 15:07:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAC31633CF;\n\tThu, 22 Aug 2024 17:07:53 +0200 (CEST)","from mail-lj1-x229.google.com (mail-lj1-x229.google.com\n\t[IPv6:2a00:1450:4864:20::229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2628F63369\n\tfor ;\n\tThu, 22 Aug 2024 17:07:52 +0200 (CEST)","by mail-lj1-x229.google.com with SMTP id\n\t38308e7fff4ca-2f3cd4ebf84so10836271fa.3\n\tfor ;\n\tThu, 22 Aug 2024 08:07:52 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"OtE/v+ws\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1724339271; x=1724944071;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=Ks5rjLcihQ3vDOihywqZmqES7l5UbVmhTCUUlrpzICg=;\n\tb=OtE/v+wsAq8/lj59f2Pa7ZKdDiUBWSIytdwPixUBDQtnyfJ5rnfkn7B4eNra2KGIFE\n\tXuVYFGk79FWSNREplO8O8KNQQ5ILDypSx2xHFLOEpax5vmbqDR242PThjrRXdv22cDR3\n\tifU/KXIPIN/N8yechCYlfYa2nS1jYMxlGXfpE=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1724339271; x=1724944071;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=Ks5rjLcihQ3vDOihywqZmqES7l5UbVmhTCUUlrpzICg=;\n\tb=fKWz+SyYR4PTmEwtl9vkOt3fwrSPi85AWHbYC489CB+Mle49vyoFfY9CosOGBP8IRI\n\teh3wy5movZu9PFHWHvbsYKQv+6tDbfV+kc+ehDsdRoULvXXRYbIA09xYbjJQ0MwnnYm6\n\tNzRjajIJRVxG5sTFh2Nr4KfOzjzNvIgH5vxED3UvTRvhdRMdo95IxogJ7mKtF2cJy4ME\n\t81X0rh78VOKEy7MAvgKaO7ECo08ag8AqMKNcsqcnnlVv1Yo10mFIMw7P6sEWPKTYrxI1\n\tvCbhXBIRixJna1Yt+1msN7St6dpq0Niy3Ffb4MXPI/vgY2eSBeHK/5AV0hb3CT/m0KYB\n\tqC1Q==","X-Gm-Message-State":"AOJu0YwplRYFkNC6bhu/GnO1opjFZWpM8FyKD9X9IT2XBeZ5zuTnsnzi\n\t/pf0irog4EYbt/u9pPwMsC0+l3XqHgseOI4DXV3wyd8g8UgEA8EbjIIwf8izfGgnm+XbuxBZAA5\n\tuPDelyVQ4t64VrI+A2rdYpSaOzzIE7Ewg+IJT","X-Google-Smtp-Source":"AGHT+IGvqZgHxi2okK1jvRqCvXGZEs+ZSRrD7UZMyGl5XAVBBp3o0ZplVWRSizuvqVzCAZX/0y78WNjecM8044jkQj0=","X-Received":"by 2002:a2e:b012:0:b0:2f4:8fb:d845 with SMTP id\n\t38308e7fff4ca-2f408fbdcabmr11309351fa.14.1724339270659;\n\tThu, 22 Aug 2024 08:07:50 -0700 (PDT)","MIME-Version":"1.0","References":"<20221220070523.3212844-1-chenghaoyang@google.com>\n\t<20221220070523.3212844-2-chenghaoyang@google.com>\n\t\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Thu, 22 Aug 2024 17:07:39 +0200","Message-ID":"","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org,\n\tHarvey Yang , \n\tChinglin Yu ","Content-Type":"multipart/alternative; boundary=\"000000000000b3476e06204705b6\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31137,"web_url":"https://patchwork.libcamera.org/comment/31137/","msgid":"","date":"2024-09-09T16:30:39","subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Harvey,\n\n \n\n> So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments\n> and fields and how to obtain the fields from the args. We can\n> instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only\n> have to specify the args, and the fields are already defined and come\n> from the class. This reduces duplication in the tracepoint definitions.\n> \n> I'm wondering from what you've implemented here that perfetto doesn't\n> support this? I don't see anything in the perfetto docs that suggest\n> that this is possible either. Although we can mitigate this issue by\n> generating the perfetto tracepoints :) (this is another reason why I\n> think we should use the lttng tracepoints definition as the main one and\n> generate the perfetto ones from those)\n> \n> \n> \n> I've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that\n> there are limited features in lttng [1], and if we use the lttng tracepoints\n> definition as the source of truth, it's hard/weird to implement some Perfetto\n> concepts, like nested slices (lifetime of trace events) and flows [2].\n\nAh ok, I see the problem.\n\n> \n> It's not impossible, as you mentioned above, that we could detect the prefix\n> and suffix of each libcamera/lttng macro. However, it's a weird code parser\n> and generator design.\n> \n> [1]: https://lttng.org/man/3/lttng-ust/v2.8/\n> [2]: https://perfetto.dev/docs/instrumentation/track-events\n> \n> \n\n \n\n> \n> \n> I agree that in the current functionalities, it's not that hard to implement\n> a homemade C/python parser, and generate Perfetto code with jinja2.\n> However, we not only \"limit\" the functionalities of Perfetto with lttng's\n> definition, but also add another layer to be maintained.\n> It's true that having duplication of libcamera tracepoint definitions\n> requires more maintenance work when updating the libcamera\n> tracepoints, while the effort to maintain the parser and code generation\n> is also non-negligible.\n> \n> I also think that creating a new language from which we generate\n> tracepoints for both is an overkill though. It seems that we don't have\n> a perfect way to support both lttng and Perfetto...\n> \n> BTW, I tried lex and yacc to parse lttng macros, and tbh it's very painful\n> that it's very hard to use lex to recognize C/C++ type as a token... So\n> yeah if we end up deciding to take this route, a homemade parser from\n> scratch makes more sense to me.\n> \n> So let me list the approaches with pros and cons:\n> 1. Generate Perfetto code from lttng macros.\n>   Pros: Easy to update libcamera tracepoint macros.\n>   Cons: Add maintenance work with the additional parser and code \n> generator; hard to implement Perfetto features that lttng doesn't have.\n\nThis makes me wonder, if Perfetto is a superset of the features of\nlttng, why not go the other way; use Perfetto tracepoints as the main\nauthority and then generate lttng tracepoints from those.\n\nWhat are your thoughts?\n\n> \n> 2. Creating a new language from which we generate both Perfetto\n> and lttng macros.\n>   Pros: Easy to update libcamera tracepoint macros.\n>   Cons: Probably an overkill; hard to cover all features from both \n> Perfetto and lttng.\n> \n> 3. As this patch implements: have duplicated libcamera tracepoint \n> definitions.\n>   Pros: Easy to add libcamera tracepoint macros to cover all features\n> from Perfetto & lttng.\n>   Cons: Developer needs to update both sides when the definition of\n> libcamera tracepoints changes; Unused macro definitions might be\n> adopted in both sides, if a feature is only available in one trace library.\n> \n> 4. Add another set of libcamera tracepoints for Perfetto, or just use \n> Perfetto macros directly in libcamera's source code.\n>   Pros: Two isolated trace stacks that don't influence each other. All\n> features can be implemented easily.\n>   Cons: Duplicated trace code in source code that might lead to \n> confusion; developers also need to update both macros when a\n> new trace is needed.\n> \n> I think approach 3rd and 4th are easier to maintain in the long\n> term. WDYT?\n> \n> Also, I'd like to know if you think Perfetto will be used in other linux\n> distributions than ChromeOS or Android.\n\nIf Perfetto gets added to the package repositories in linux\ndistributions then I think it would be used :)\n\nNot being able to sudo apt install perfetto is a pretty big blocker.\n\n\nThanks,\n\n\nPaul\n\n> \n> Chinglin, please help amend anything I missed as well :)\n> \n> BR,\n> Harvey\n> \n> \n> [2] utils/ipc/meson.build\n> [3] include/libcamera/ipa/meson.build\n> [4] src/libcamera/ipa/meson.build\n> \n> Would you be able to do this?\n> \n> \n> Paul\n> \n> > diff --git a/src/libcamera/tracepoints.cpp b/src/libcamera/\n> tracepoints.cpp\n> > index 0173b75a..a07ea531 100644\n> > --- a/src/libcamera/tracepoints.cpp\n> > +++ b/src/libcamera/tracepoints.cpp\n> > @@ -4,7 +4,18 @@\n> >   *\n> >   * tracepoints.cpp - Tracepoints with lttng\n> >   */\n> > +\n> > +#if HAVE_PERFETTO\n> > +\n> > +#include \"libcamera/internal/tracepoints.h\"\n> > +\n> > +PERFETTO_TRACK_EVENT_STATIC_STORAGE();\n> > +\n> > +#else\n> > +\n> >  #define TRACEPOINT_CREATE_PROBES\n> >  #define TRACEPOINT_DEFINE\n> > \n> >  #include \"libcamera/internal/tracepoints.h\"\n> > +\n> > +#endif /* HAVE_PERFETTO */\n> > --\n> > 2.39.0.314.g84b9a713c41-goog\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5CFF0BF415\n\tfor ;\n\tMon, 9 Sep 2024 16:30:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 17452634F2;\n\tMon, 9 Sep 2024 18:30:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18CFE634EB\n\tfor ;\n\tMon, 9 Sep 2024 18:30:44 +0200 (CEST)","from pyrite.rasen.tech (213-229-8-243.static.upcbusiness.at\n\t[213.229.8.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 75DD85A4;\n\tMon, 9 Sep 2024 18:29:27 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ISNzjFWn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725899367;\n\tbh=FIoG5Mf30/IRV+D1lmH99hd4mKypXWN+amhHyZr+6o8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ISNzjFWnhVccIR270iBI25oRo7zUTn4Fum22cNcTcw2kJlQgu93d7X7GzcvdLUkpO\n\t/R6OelEUKEWAGs0lPnxFhpylLF14xi1jtDIwdzVr80K+V6eu0wcXm+2zfqcQsKj8vL\n\tpgJf6lHRhxXrqVaer2gJCdKKAiOP7eZ+712upfpQ=","Date":"Mon, 9 Sep 2024 18:30:39 +0200","From":"Paul Elder ","To":"Cheng-Hao Yang ","Cc":"Chinglin Yu ,\n\tlibcamera-devel@lists.libcamera.org, \n\tHarvey Yang ","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","Message-ID":"","References":"<20221220070523.3212844-1-chenghaoyang@google.com>\n\t<20221220070523.3212844-2-chenghaoyang@google.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31165,"web_url":"https://patchwork.libcamera.org/comment/31165/","msgid":"","date":"2024-09-11T10:05:18","subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Paul,\n\nOn Tue, Sep 10, 2024 at 12:30 AM Paul Elder \nwrote:\n\n> Hi Harvey,\n>\n> \n>\n> > So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments\n> > and fields and how to obtain the fields from the args. We can\n> > instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only\n> > have to specify the args, and the fields are already defined and come\n> > from the class. This reduces duplication in the tracepoint\n> definitions.\n> >\n> > I'm wondering from what you've implemented here that perfetto doesn't\n> > support this? I don't see anything in the perfetto docs that suggest\n> > that this is possible either. Although we can mitigate this issue by\n> > generating the perfetto tracepoints :) (this is another reason why I\n> > think we should use the lttng tracepoints definition as the main one\n> and\n> > generate the perfetto ones from those)\n> >\n> >\n> >\n> > I've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that\n> > there are limited features in lttng [1], and if we use the lttng\n> tracepoints\n> > definition as the source of truth, it's hard/weird to implement some\n> Perfetto\n> > concepts, like nested slices (lifetime of trace events) and flows [2].\n>\n> Ah ok, I see the problem.\n>\n> >\n> > It's not impossible, as you mentioned above, that we could detect the\n> prefix\n> > and suffix of each libcamera/lttng macro. However, it's a weird code\n> parser\n> > and generator design.\n> >\n> > [1]: https://lttng.org/man/3/lttng-ust/v2.8/\n> > [2]: https://perfetto.dev/docs/instrumentation/track-events\n> >\n> >\n>\n> \n>\n> >\n> >\n> > I agree that in the current functionalities, it's not that hard to\n> implement\n> > a homemade C/python parser, and generate Perfetto code with jinja2.\n> > However, we not only \"limit\" the functionalities of Perfetto with lttng's\n> > definition, but also add another layer to be maintained.\n> > It's true that having duplication of libcamera tracepoint definitions\n> > requires more maintenance work when updating the libcamera\n> > tracepoints, while the effort to maintain the parser and code generation\n> > is also non-negligible.\n> >\n> > I also think that creating a new language from which we generate\n> > tracepoints for both is an overkill though. It seems that we don't have\n> > a perfect way to support both lttng and Perfetto...\n> >\n> > BTW, I tried lex and yacc to parse lttng macros, and tbh it's very\n> painful\n> > that it's very hard to use lex to recognize C/C++ type as a token... So\n> > yeah if we end up deciding to take this route, a homemade parser from\n> > scratch makes more sense to me.\n> >\n> > So let me list the approaches with pros and cons:\n> > 1. Generate Perfetto code from lttng macros.\n> > Pros: Easy to update libcamera tracepoint macros.\n> > Cons: Add maintenance work with the additional parser and code\n> > generator; hard to implement Perfetto features that lttng doesn't have.\n>\n> This makes me wonder, if Perfetto is a superset of the features of\n> lttng, why not go the other way; use Perfetto tracepoints as the main\n> authority and then generate lttng tracepoints from those.\n>\n> What are your thoughts?\n>\n\nI don't think Perfetto MACROs is a superset of lttng MACROs. If we go\nwith this approach, there are some features dropped (like integer_hex\nand enum, and the concept of TP_ARGS), and the code would be messy.\nTake request_complete_buffer as the example:\n\n```\n# Perfetto\nTRACE_EVENT(\"libcamera\", \"request_complete_buffer\",\n \"request_private_ptr\", reinterpret_cast(req),\n \"cookie\", req->_o()->cookie(),\n \"status\", req->_o()->status(),\n \"buffer_ptr\", reinterpret_cast(buf),\n \"buffer_status\", buf->metadata().status);\n```\n\nIt'd be how perfetto implements the macro. Originally lttng has only two\ninputs: `libcamera::Request::internal *req` and\n`libcamera::FrameBuffer *buf`. Unless we're going to do complicated\ntype diagnosis and simplification, we might end up with having five\ninputs as well. Also, I don't know how to indicate the usage of\nctf_integer_hex (for `req` and `buf`) and ctf_enum (for status).\n\nI'd still prefer the 3rd or 4th approach.\n\n\n>\n> >\n> > 2. Creating a new language from which we generate both Perfetto\n> > and lttng macros.\n> > Pros: Easy to update libcamera tracepoint macros.\n> > Cons: Probably an overkill; hard to cover all features from both\n> > Perfetto and lttng.\n> >\n> > 3. As this patch implements: have duplicated libcamera tracepoint\n> > definitions.\n> > Pros: Easy to add libcamera tracepoint macros to cover all features\n> > from Perfetto & lttng.\n> > Cons: Developer needs to update both sides when the definition of\n> > libcamera tracepoints changes; Unused macro definitions might be\n> > adopted in both sides, if a feature is only available in one trace\n> library.\n> >\n> > 4. Add another set of libcamera tracepoints for Perfetto, or just use\n> > Perfetto macros directly in libcamera's source code.\n> > Pros: Two isolated trace stacks that don't influence each other. All\n> > features can be implemented easily.\n> > Cons: Duplicated trace code in source code that might lead to\n> > confusion; developers also need to update both macros when a\n> > new trace is needed.\n> >\n> > I think approach 3rd and 4th are easier to maintain in the long\n> > term. WDYT?\n> >\n> > Also, I'd like to know if you think Perfetto will be used in other linux\n> > distributions than ChromeOS or Android.\n>\n> If Perfetto gets added to the package repositories in linux\n> distributions then I think it would be used :)\n>\n> Not being able to sudo apt install perfetto is a pretty big blocker.\n>\n>\nYeah I understand, while Perfetto is self-contained and simple to build\nin linux distros. May we consider testing it on the gitlab CI, after we\nsomehow add the support?\n\nBR,\nHarvey\n\n\n>\n> Thanks,\n>\n>\n> Paul\n>\n> >\n> > Chinglin, please help amend anything I missed as well :)\n> >\n> > BR,\n> > Harvey\n> >\n> >\n> > [2] utils/ipc/meson.build\n> > [3] include/libcamera/ipa/meson.build\n> > [4] src/libcamera/ipa/meson.build\n> >\n> > Would you be able to do this?\n> >\n> >\n> > Paul\n> >\n> > > diff --git a/src/libcamera/tracepoints.cpp b/src/libcamera/\n> > tracepoints.cpp\n> > > index 0173b75a..a07ea531 100644\n> > > --- a/src/libcamera/tracepoints.cpp\n> > > +++ b/src/libcamera/tracepoints.cpp\n> > > @@ -4,7 +4,18 @@\n> > > *\n> > > * tracepoints.cpp - Tracepoints with lttng\n> > > */\n> > > +\n> > > +#if HAVE_PERFETTO\n> > > +\n> > > +#include \"libcamera/internal/tracepoints.h\"\n> > > +\n> > > +PERFETTO_TRACK_EVENT_STATIC_STORAGE();\n> > > +\n> > > +#else\n> > > +\n> > > #define TRACEPOINT_CREATE_PROBES\n> > > #define TRACEPOINT_DEFINE\n> > >\n> > > #include \"libcamera/internal/tracepoints.h\"\n> > > +\n> > > +#endif /* HAVE_PERFETTO */\n> > > --\n> > > 2.39.0.314.g84b9a713c41-goog\n> > >\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 20D09BF415\n\tfor ;\n\tWed, 11 Sep 2024 10:05:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BD8CB618FB;\n\tWed, 11 Sep 2024 12:05:32 +0200 (CEST)","from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com\n\t[IPv6:2a00:1450:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 77032618F2\n\tfor ;\n\tWed, 11 Sep 2024 12:05:30 +0200 (CEST)","by mail-lj1-x22b.google.com with SMTP id\n\t38308e7fff4ca-2f763e9e759so18582641fa.3\n\tfor ;\n\tWed, 11 Sep 2024 03:05:30 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"WF/yRUtt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1726049129; x=1726653929;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=2ufC3m9N//qky80I9z51OTcgwALIsG3nGf6KxNlgiBo=;\n\tb=WF/yRUttrVtqZSyx/ocZMrSqS3vy5gUdjLXRlRlwCXvG3cAx5LNwl5hwks8n8jNYw0\n\t7KSQ9Z+cyvJASN7DeKKaXmF/T61K6wNsbUk88Rrzp0l4TSaZV0uThZxYLGnDoujgRYGS\n\tm0iK/H/8p0rV37bTZk9/s2lpkY1PVAhJEOTWY=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1726049129; x=1726653929;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=2ufC3m9N//qky80I9z51OTcgwALIsG3nGf6KxNlgiBo=;\n\tb=T2yKc8DZFujQbOShnvXzv/1HKfDevopJ3eGSzFDYG5oBZaILkUeJ4Imgi2fc9XTrwt\n\txqC6o7ReZU8ynToUt5tQ4357oIiGJ4FHNjV3dZ1QUCPP//FJjoYrOTTZWr/vaGuI43cf\n\tyNbd5G/mTkvR6giFpBcv+aLTTPCIfyf/1miKWoUDGoUEPekrPXtwFsXyiAnwgWPWKqbt\n\tvD2CBjeEiBgBwr3BVbzLUhmHD7N5I479YioWcSUwN2ZFucUWz3YdtTMD8Plj1JIhbObu\n\tEzArhccgGveqTSPB8aXgZrJ1Kgpzt/UrA4TC7j0d6oTSfLhwtekoS+AO7uvpAWng+oFG\n\tJ05Q==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCXeLNJl2P/q71reyGNDf2TSkIzhflpsnFwtOVxiQv+nyBYgl+SFg+iZyLCNdd1jCOiZNBZR7Kink7PBSX9Fv3w=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YyOlr1jnvQTE7hxXSi793/5r/1LzhDqE0NqlGRN7cOdoQoLbnAl\n\ttY+kLj3W+gD1uBE7MlyYLRxJEMchu5tWCevsy+j+rh72Zr0o9JWnN+vwIDit4ufTVQUZb/XxOTO\n\t6JlgBR4Y09ld6w5qg3kNqbrjZaznWTlXiRosR","X-Google-Smtp-Source":"AGHT+IHov5fUDdN0RSeNjgwZDrT3IPDQKZrFACLWJA+AsegJAeryhWt9Zn/8Bej6FATasiNpx8CbwzdlveI4Nf0WL2o=","X-Received":"by 2002:a05:651c:545:b0:2f6:649e:bf5c with SMTP id\n\t38308e7fff4ca-2f77b75241emr16694201fa.17.1726049129043;\n\tWed, 11 Sep 2024 03:05:29 -0700 (PDT)","MIME-Version":"1.0","References":"<20221220070523.3212844-1-chenghaoyang@google.com>\n\t<20221220070523.3212844-2-chenghaoyang@google.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Wed, 11 Sep 2024 18:05:18 +0800","Message-ID":"","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","To":"Paul Elder ","Cc":"Chinglin Yu ,\n\tlibcamera-devel@lists.libcamera.org, \n\tHarvey Yang ","Content-Type":"multipart/alternative; boundary=\"00000000000033b38f0621d5210e\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31167,"web_url":"https://patchwork.libcamera.org/comment/31167/","msgid":"<20240911111011.GC4470@pendragon.ideasonboard.com>","date":"2024-09-11T11:10:11","subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Harvey,\n\nOn Wed, Sep 11, 2024 at 06:05:18PM +0800, Cheng-Hao Yang wrote:\n> On Tue, Sep 10, 2024 at 12:30 AM Paul Elder wrote:\n> > Hi Harvey,\n> >\n> > \n> >\n> > > So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines arguments\n> > > and fields and how to obtain the fields from the args. We can\n> > > instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE, where we only\n> > > have to specify the args, and the fields are already defined and come\n> > > from the class. This reduces duplication in the tracepoint definitions.\n> > >\n> > > I'm wondering from what you've implemented here that perfetto doesn't\n> > > support this? I don't see anything in the perfetto docs that suggest\n> > > that this is possible either. Although we can mitigate this issue by\n> > > generating the perfetto tracepoints :) (this is another reason why I\n> > > think we should use the lttng tracepoints definition as the main one and\n> > > generate the perfetto ones from those)\n> > >\n> > > I've discussed with ChromeOS Perfetto TL chinglinyu@, and we found that\n> > > there are limited features in lttng [1], and if we use the lttng tracepoints\n> > > definition as the source of truth, it's hard/weird to implement some Perfetto\n> > > concepts, like nested slices (lifetime of trace events) and flows [2].\n> >\n> > Ah ok, I see the problem.\n> >\n> > > It's not impossible, as you mentioned above, that we could detect the prefix\n> > > and suffix of each libcamera/lttng macro. However, it's a weird code parser\n> > > and generator design.\n> > >\n> > > [1]: https://lttng.org/man/3/lttng-ust/v2.8/\n> > > [2]: https://perfetto.dev/docs/instrumentation/track-events\n> >\n> > \n> >\n> > > I agree that in the current functionalities, it's not that hard to implement\n> > > a homemade C/python parser, and generate Perfetto code with jinja2.\n> > > However, we not only \"limit\" the functionalities of Perfetto with lttng's\n> > > definition, but also add another layer to be maintained.\n> > > It's true that having duplication of libcamera tracepoint definitions\n> > > requires more maintenance work when updating the libcamera\n> > > tracepoints, while the effort to maintain the parser and code generation\n> > > is also non-negligible.\n> > >\n> > > I also think that creating a new language from which we generate\n> > > tracepoints for both is an overkill though. It seems that we don't have\n> > > a perfect way to support both lttng and Perfetto...\n> > >\n> > > BTW, I tried lex and yacc to parse lttng macros, and tbh it's very painful\n> > > that it's very hard to use lex to recognize C/C++ type as a token... So\n> > > yeah if we end up deciding to take this route, a homemade parser from\n> > > scratch makes more sense to me.\n> > >\n> > > So let me list the approaches with pros and cons:\n> > > 1. Generate Perfetto code from lttng macros.\n> > > Pros: Easy to update libcamera tracepoint macros.\n> > > Cons: Add maintenance work with the additional parser and code\n> > > generator; hard to implement Perfetto features that lttng doesn't have.\n> >\n> > This makes me wonder, if Perfetto is a superset of the features of\n> > lttng, why not go the other way; use Perfetto tracepoints as the main\n> > authority and then generate lttng tracepoints from those.\n> >\n> > What are your thoughts?\n> \n> I don't think Perfetto MACROs is a superset of lttng MACROs. If we go\n> with this approach, there are some features dropped (like integer_hex\n> and enum, and the concept of TP_ARGS), and the code would be messy.\n> Take request_complete_buffer as the example:\n> \n> ```\n> # Perfetto\n> TRACE_EVENT(\"libcamera\", \"request_complete_buffer\",\n> \"request_private_ptr\", reinterpret_cast(req),\n> \"cookie\", req->_o()->cookie(),\n> \"status\", req->_o()->status(),\n> \"buffer_ptr\", reinterpret_cast(buf),\n> \"buffer_status\", buf->metadata().status);\n> ```\n> \n> It'd be how perfetto implements the macro. Originally lttng has only two\n> inputs: `libcamera::Request::internal *req` and\n> `libcamera::FrameBuffer *buf`. Unless we're going to do complicated\n> type diagnosis and simplification, we might end up with having five\n> inputs as well. Also, I don't know how to indicate the usage of\n> ctf_integer_hex (for `req` and `buf`) and ctf_enum (for status).\n> \n> I'd still prefer the 3rd or 4th approach.\n\nThe 4th option, duplicating trace points in the libcamera sources, is\nthe one I like the least. The 3rd option is also something I don't like\nmuch. It means that anyone adding trace points will need to do extra\nwork to support two trace systems. Testing it locally will be hard, even\nfor build testing. We could possibly handle the build testing with CI\n(see below), even if that will be way less convenient that doing local\nbuilds. Runtime testing is likely out of question.\n\nI'm not comfortable with the amount of extra work this will cause to\neverybody to support a Google-only trace system. I'm willing to find a\ncompromise though, if Google could do the necessary work needed to\npackage Perfetto in distributions, I could accept extra work on our side\ntoo.\n\n> > > 2. Creating a new language from which we generate both Perfetto\n> > > and lttng macros.\n> > > Pros: Easy to update libcamera tracepoint macros.\n> > > Cons: Probably an overkill; hard to cover all features from both\n> > > Perfetto and lttng.\n> > >\n> > > 3. As this patch implements: have duplicated libcamera tracepoint\n> > > definitions.\n> > > Pros: Easy to add libcamera tracepoint macros to cover all features\n> > > from Perfetto & lttng.\n> > > Cons: Developer needs to update both sides when the definition of\n> > > libcamera tracepoints changes; Unused macro definitions might be\n> > > adopted in both sides, if a feature is only available in one trace\n> > library.\n> > >\n> > > 4. Add another set of libcamera tracepoints for Perfetto, or just use\n> > > Perfetto macros directly in libcamera's source code.\n> > > Pros: Two isolated trace stacks that don't influence each other. All\n> > > features can be implemented easily.\n> > > Cons: Duplicated trace code in source code that might lead to\n> > > confusion; developers also need to update both macros when a\n> > > new trace is needed.\n> > >\n> > > I think approach 3rd and 4th are easier to maintain in the long\n> > > term. WDYT?\n> > >\n> > > Also, I'd like to know if you think Perfetto will be used in other linux\n> > > distributions than ChromeOS or Android.\n> >\n> > If Perfetto gets added to the package repositories in linux\n> > distributions then I think it would be used :)\n> >\n> > Not being able to sudo apt install perfetto is a pretty big blocker.\n>\n> Yeah I understand, while Perfetto is self-contained and simple to build\n> in linux distros.\n\nDo you mean by running the tools/install-build-deps script that\ndownloads toolchain binaries from google servers ? That doesn't really\nmatch the philosophy of Linux distributions.\n\n> May we consider testing it on the gitlab CI, after we\n> somehow add the support?\n\nIf we add support for it I certainly would like to see it being tested\nin CI, yes. After a quick look at the Perfetto build system, it looks\nlike adding Perfetto to the CI docker containers may not be trivial.\nSomeone will need to volunteer.\n\n> > > Chinglin, please help amend anything I missed as well :)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id EE66ABF415\n\tfor ;\n\tWed, 11 Sep 2024 11:10:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B32E5634F9;\n\tWed, 11 Sep 2024 13:10:47 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EFA67618F2\n\tfor ;\n\tWed, 11 Sep 2024 13:10:45 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(213-229-8-243.static.upcbusiness.at [213.229.8.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 15030BEB;\n\tWed, 11 Sep 2024 13:09:28 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LLREG/bp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1726052968;\n\tbh=gXQa3drhBVUUDorvt0PeQ+YEFZkPv6HdDG0229kgFSM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LLREG/bpttGvZ1ti+sUSOHBsdSlWchIrwDxQUDv6Ovg2tiLCUMhqA7ZNL2MtQCgEg\n\tS2p38xCB7lioDeOG6o5/tgd+hsRO66/Q7OhEVJKscMxdYU+3mVbIjq6alwkBlNKWa9\n\tjLLWZMTBJ/M9eobrj+aJX+A3BfJ/1uHuvlRiBdpU=","Date":"Wed, 11 Sep 2024 14:10:11 +0300","From":"Laurent Pinchart ","To":"Cheng-Hao Yang ","Cc":"Paul Elder ,\n\tChinglin Yu ,\n\tlibcamera-devel@lists.libcamera.org, \n\tHarvey Yang ","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","Message-ID":"<20240911111011.GC4470@pendragon.ideasonboard.com>","References":"<20221220070523.3212844-1-chenghaoyang@google.com>\n\t<20221220070523.3212844-2-chenghaoyang@google.com>\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31259,"web_url":"https://patchwork.libcamera.org/comment/31259/","msgid":"","date":"2024-09-18T07:57:06","subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi libcamera developers,\n\nI've consulted with Google Perfetto team, and yes, we agree\nthat it makes more sense to have one debugging tool as the\nsource of truth and generate code for the other tool, to ease\nthe development effort in the future.\n\nPlease also share your concerns on the two approaches:\n\nOn Sat, Sep 14, 2024 at 1:14 AM Lalit Maganti wrote:\n\n>\n>\n> On Fri, 13 Sept 2024 at 18:08, Cheng-Hao Yang \n> wrote:\n>\n>> Thank you Lalit for the quick and thorough response,\n>>\n>> Do you mind if I directly forward your messages to libcamera upstream\n>> directly?\n>> I'll wait for a couple of days until everyone has the chance to speak\n>> out, and do a brief summary though.\n>>\n>\n> Sure that sounds good.\n>\n>\n> Responses inline:\n>>\n>> On Fri, Sep 13, 2024 at 6:19 PM Lalit Maganti wrote:\n>>\n>>> Also +Daniele Di Proietto is the C SDK better\n>>> at being closer to LTTng in ethos? I know it allows e.g. defining custom\n>>> protos directly in code and writing out those? Would that be a better fit\n>>> for what these folks are trying to do?\n>>>\n>>\n>> Yeah I forgot to mention that at least in CrOS, we're using cros-built\n>> Perfetto C SDK in libcamera.\n>>\n>>\n>>>\n>>> On Fri, 13 Sept 2024, 10:54 Lalit Maganti, wrote:\n>>>\n>>>> Please do feel free to setup some time with us (you can book slots at\n>>>> go/perfetto-office-hours\n>>>> ) or reach out by IM\n>>>> btw to speed iteration on this up if you have more questions/want to\n>>>> discuss more.\n>>>>\n>>>> On Fri, 13 Sept 2024 at 10:53, Lalit Maganti wrote:\n>>>>\n>>>>> Hi Harvey,\n>>>>>\n>>>>> Responses inline.\n>>>>>\n>>>>> On Fri, 13 Sept 2024 at 08:56, Cheng-Hao Yang \n>>>>> wrote:\n>>>>>\n>>>>>> Hi Primiano and Perfetto developers,\n>>>>>>\n>>>>>> This is Harvey, working in CrOS Camera team on libcamera project\n>>>>>> . I need some help from you, as I'm working\n>>>>>> on supporting Perfetto in libcamera upstream, which currently uses lttng as\n>>>>>> the tracing tool. Libcamera upstream has the concern that Perfetto is not\n>>>>>> added in the package repositories in linux distributions, and the way to\n>>>>>> build it in linux doesn't match the philosophy of Linux distributions.\n>>>>>>\n>>>>>> We're considering supporting both tracing tools in libcamera, while\n>>>>>> libcamera upstream strongly prefers to have only one source of truth, which\n>>>>>> means to maintain only one version of trace macros/configs, and generate\n>>>>>> macros for the other (or both of them). This email thread contains the\n>>>>>> patch of 3rd approach\n>>>>>> listed below.\n>>>>>>\n>>>>>> I need your help on mainly two things:\n>>>>>> 1. An official reply to confirm that it is or isn't a priority to add\n>>>>>> Perfetto in the package repositories in linux distributions (I assume it\n>>>>>> isn't though). After providing this info, I can confirm with libcamera\n>>>>>> upstream our consensus of Perfetto support and responsibilities in the\n>>>>>> future.\n>>>>>>\n>>>>>\n>>>>> It is not a priority (and historically not been a priority) *but* at\n>>>>> least for Debian we already did a bunch of the work to make a package for\n>>>>> distributing Perfetto (see here\n>>>>> ).\n>>>>> If someone from your side was willing to do the work to submit this\n>>>>> package, we'd be happy to keep it updated and maintained going forward.\n>>>>>\n>>>>> So for us:\n>>>>>\n>>>>> 1. Is someone from your side willing to take the work we've done\n>>>>> and push it to completion?\n>>>>> 2. Are there any other distros apart from Debian which libcamera\n>>>>> would need us to be in?\n>>>>>\n>>>>> 1. Unfortunately, I don't think we have the bandwidth to help push it\n>> to completion in the current stage...\n>> 2. I believe there are at least Fedora and Raspberry. I'll let the\n>> libcamera upstream developers correct me.\n>>\n>\n> That's fair. I think in that case, it would be difficult for us to get it\n> supported to the level that they want as well unfortunately. We're already\n> resource constrained as it is and with Android and AL work, I don't think\n> we'll have the bandwidth to deal with all the work for upstreaming into\n> debian and all the other distros.\n>\n>\n>>>>>\n>>>>>> 2. Take a brief look at the approaches below, and check if I missed\n>>>>>> something. You may focus on the 1st or 5th, which I'm currently inclined\n>>>>>> with.\n>>>>>>\n>>>>>\n>>>>>> Here's the list of approaches that we can think of:\n>>>>>>\n>>>>>> 1. Generate Perfetto code from lttng macros.\n>>>>>> Pros: Easy to update libcamera tracepoint macros.\n>>>>>> Cons: Add maintenance work with the additional parser and code\n>>>>>> generator; hard to implement Perfetto features that lttng doesn't\n>>>>>> have.\n>>>>>>\n>>>>>\n>>>>> An option but as you say, it does not support a bunch of features in\n>>>>> Perfetto which LTTNG does not have.\n>>>>>\n>>>>\nThis one, which you might prefer.\n\n\n>\n>>>>>\n>>>>>>\n>>>>>> 2. Create a new language/config from which we generate both Perfetto\n>>>>>> and lttng macros.\n>>>>>> Pros: Easy to update libcamera tracepoint macros.\n>>>>>> Cons: Probably an overkill; hard to cover all features from both\n>>>>>> Perfetto and lttng.\n>>>>>>\n>>>>>\n>>>>> Yeah would not go here.\n>>>>>\n>>>>> 3. As this patch implements: have duplicated libcamera tracepoint\n>>>>>> definitions.\n>>>>>> Pros: Easy to add libcamera tracepoint macros to cover all features\n>>>>>> from Perfetto & lttng.\n>>>>>> Cons: Developer needs to update both sides when the definition of\n>>>>>> libcamera tracepoints changes; Unused macro definitions might be\n>>>>>> adopted in both sides, if a feature is only available in one trace\n>>>>>> library.\n>>>>>>\n>>>>>\n>>>>> Inevitably things would get out of sync, I can see why they are\n>>>>> against this.\n>>>>>\n>>>>> 4. Add another set of libcamera tracepoints for Perfetto, or just use\n>>>>>> Perfetto macros directly in libcamera's source code.\n>>>>>> Pros: Two isolated trace stacks that don't influence each other. All\n>>>>>> features can be implemented easily.\n>>>>>> Cons: Duplicated trace code in source code that might lead to\n>>>>>> confusion; developers also need to update both macros when a\n>>>>>> new trace is needed.\n>>>>>>\n>>>>>\n>>>>> As above, same deal.\n>>>>>\n>>>>>\n>>>>>> 5. Add libcamera macro usages, and generate lttng macros with codegen.\n>>>>>> Pros: It supports all perfetto features.\n>>>>>> Cons: Some lttng-only features (like ctf_integer_hex and ctf_enum)\n>>>>>> are not available without weird hacks.\n>>>>>>\n>>>>>\n>>>>> If there are things like this, I think we'd be happy to work with you\n>>>>> on getting those gaps resolved (if it's feasible, I haven't looked into the\n>>>>> details in this particular case, it might not be).\n>>>>>\n>>>>\n>> Yeah it might be the best case for us now.\n>>\n>\n> Cool sounds good :)\n>\n\nOr this one, that Google prefers. We might need to find\na way to support hex & enum in this case though.\n\n\n>\n> --------------\n>>>>>>\n>>>>>> As libcamera upstream has a strong opinion, I might end up going with\n>>>>>> the 1st or 5th approach. The reason to consider the 1st is that currently\n>>>>>> the libcamera trace points might be better off with Perfetto counters (or\n>>>>>> slices with minimum lifetime), instead of slices. Please correct me if I'm\n>>>>>> wrong, but I think Perfetto slices are most useful for nested function\n>>>>>> calls. However, a lot of libcamera operations are triggered by callbacks\n>>>>>> (in libcamera, Signal).\n>>>>>> I tried Perfetto flow yesterday, and it works on libcamera. We can\n>>>>>> possibly add a hack in the potential codegen to set it up though.\n>>>>>>\n>>>>>\n>>>>> Perfetto slices are not just for sync function calls: you can create\n>>>>> synthetic sync stacks from them based on callback lifecycles: in\n>>>>> Chrome/ChromeOS where everything is based on TaskRunner (thread loops) and\n>>>>> callbacks, Perfetto is used very extensively and along with flows, allows\n>>>>> visualizing async stuff just fine.\n>>>>>\n>>>>> Counters are not a good fit for visualizing latencies of things which\n>>>>> given the nature of libcamera is almost certainly what you want.\n>>>>>\n>>>>\n>> Thanks for the info! Flow is very useful indeed. I'll do some more\n>> studies on the synthetic sync stacks, and check the best way to add traces\n>> in libcamera. It might be very different from the current lttng ones\n>> though, I believe.\n>>\n>\n> I mean an option here is to do LTTNG -> Perfetto conversion and then add\n> some custom instrumentation in the places where it really helps having\n> custom Perfetto instrumentation maybe? Maybe the libcamera folk would be\n> more amenable to that?\n>\n>\n>>\n>>>>> Let me know if there's anything I haven't clarified yet.\n>>>>>>\n>>>>>> Many thanks!\n>>>>>> Harvey\n>>>>>>\n>>>>>> ---------- Forwarded message ---------\n>>>>>> From: Laurent Pinchart \n>>>>>> Date: Wed, Sep 11, 2024 at 7:10 PM\n>>>>>> Subject: Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with\n>>>>>> perfetto in ChromeOS\n>>>>>> To: Cheng-Hao Yang \n>>>>>> Cc: Paul Elder , Chinglin Yu <\n>>>>>> chinglinyu@chromium.org>, ,\n>>>>>> Harvey Yang \n>>>>>>\n>>>>>>\n>>>>>> Hi Harvey,\n>>>>>>\n>>>>>> On Wed, Sep 11, 2024 at 06:05:18PM +0800, Cheng-Hao Yang wrote:\n>>>>>> > On Tue, Sep 10, 2024 at 12:30 AM Paul Elder wrote:\n>>>>>> > > Hi Harvey,\n>>>>>> > >\n>>>>>> > > \n>>>>>> > >\n>>>>>> > > > So in lttng we have a TRACEPOINT_EVENT_CLASS, which defines\n>>>>>> arguments\n>>>>>> > > > and fields and how to obtain the fields from the args. We\n>>>>>> can\n>>>>>> > > > instantiate tracepoints with TRACEPOINT_EVENT_INSTANCE,\n>>>>>> where we only\n>>>>>> > > > have to specify the args, and the fields are already\n>>>>>> defined and come\n>>>>>> > > > from the class. This reduces duplication in the tracepoint\n>>>>>> definitions.\n>>>>>> > > >\n>>>>>> > > > I'm wondering from what you've implemented here that\n>>>>>> perfetto doesn't\n>>>>>> > > > support this? I don't see anything in the perfetto docs\n>>>>>> that suggest\n>>>>>> > > > that this is possible either. Although we can mitigate this\n>>>>>> issue by\n>>>>>> > > > generating the perfetto tracepoints :) (this is another\n>>>>>> reason why I\n>>>>>> > > > think we should use the lttng tracepoints definition as the\n>>>>>> main one and\n>>>>>> > > > generate the perfetto ones from those)\n>>>>>>\n>>>>>\n>>>>> The LTTNG model is very very different from the Perfetto model. For\n>>>>> us, the equivalent of LTTNG \"tracepoints\" are defined directly in Perfetto\n>>>>> source code. You wouldn't see it on the libcamera side as you use the\n>>>>> macros to directly write these (they're really protos) without having a\n>>>>> \"definition\" in the libcamera source code.\n>>>>>\n>>>>>\n>>>>>> > > >\n>>>>>> > > > I've discussed with ChromeOS Perfetto TL chinglinyu@, and we\n>>>>>> found that\n>>>>>> > > > there are limited features in lttng [1], and if we use the\n>>>>>> lttng tracepoints\n>>>>>> > > > definition as the source of truth, it's hard/weird to implement\n>>>>>> some Perfetto\n>>>>>> > > > concepts, like nested slices (lifetime of trace events) and\n>>>>>> flows [2].\n>>>>>> > >\n>>>>>> > > Ah ok, I see the problem.\n>>>>>> > >\n>>>>>> > > > It's not impossible, as you mentioned above, that we could\n>>>>>> detect the prefix\n>>>>>> > > > and suffix of each libcamera/lttng macro. However, it's a weird\n>>>>>> code parser\n>>>>>> > > > and generator design.\n>>>>>> > > >\n>>>>>> > > > [1]: https://lttng.org/man/3/lttng-ust/v2.8/\n>>>>>> > > > [2]: https://perfetto.dev/docs/instrumentation/track-events\n>>>>>> > >\n>>>>>> > > \n>>>>>> > >\n>>>>>> > > > I agree that in the current functionalities, it's not that hard\n>>>>>> to implement\n>>>>>> > > > a homemade C/python parser, and generate Perfetto code with\n>>>>>> jinja2.\n>>>>>> > > > However, we not only \"limit\" the functionalities of Perfetto\n>>>>>> with lttng's\n>>>>>> > > > definition, but also add another layer to be maintained.\n>>>>>> > > > It's true that having duplication of libcamera tracepoint\n>>>>>> definitions\n>>>>>> > > > requires more maintenance work when updating the libcamera\n>>>>>> > > > tracepoints, while the effort to maintain the parser and code\n>>>>>> generation\n>>>>>> > > > is also non-negligible.\n>>>>>> > > >\n>>>>>> > > > I also think that creating a new language from which we generate\n>>>>>> > > > tracepoints for both is an overkill though. It seems that we\n>>>>>> don't have\n>>>>>> > > > a perfect way to support both lttng and Perfetto...\n>>>>>> > > >\n>>>>>> > > > BTW, I tried lex and yacc to parse lttng macros, and tbh it's\n>>>>>> very painful\n>>>>>> > > > that it's very hard to use lex to recognize C/C++ type as a\n>>>>>> token... So\n>>>>>> > > > yeah if we end up deciding to take this route, a homemade\n>>>>>> parser from\n>>>>>> > > > scratch makes more sense to me.\n>>>>>> > > >\n>>>>>> > > > So let me list the approaches with pros and cons:\n>>>>>> > > > 1. Generate Perfetto code from lttng macros.\n>>>>>> > > > Pros: Easy to update libcamera tracepoint macros.\n>>>>>> > > > Cons: Add maintenance work with the additional parser and code\n>>>>>> > > > generator; hard to implement Perfetto features that lttng\n>>>>>> doesn't have.\n>>>>>> > >\n>>>>>> > > This makes me wonder, if Perfetto is a superset of the features of\n>>>>>> > > lttng, why not go the other way; use Perfetto tracepoints as the\n>>>>>> main\n>>>>>> > > authority and then generate lttng tracepoints from those.\n>>>>>> > >\n>>>>>> > > What are your thoughts?\n>>>>>> >\n>>>>>> > I don't think Perfetto MACROs is a superset of lttng MACROs. If we\n>>>>>> go\n>>>>>> > with this approach, there are some features dropped (like\n>>>>>> integer_hex\n>>>>>> > and enum, and the concept of TP_ARGS), and the code would be messy.\n>>>>>> > Take request_complete_buffer as the example:\n>>>>>> >\n>>>>>> > ```\n>>>>>> > # Perfetto\n>>>>>> > TRACE_EVENT(\"libcamera\", \"request_complete_buffer\",\n>>>>>> > \"request_private_ptr\", reinterpret_cast(req),\n>>>>>> > \"cookie\", req->_o()->cookie(),\n>>>>>> > \"status\", req->_o()->status(),\n>>>>>> > \"buffer_ptr\", reinterpret_cast(buf),\n>>>>>> > \"buffer_status\", buf->metadata().status);\n>>>>>> > ```\n>>>>>> >\n>>>>>> > It'd be how perfetto implements the macro. Originally lttng has\n>>>>>> only two\n>>>>>> > inputs: `libcamera::Request::internal *req` and\n>>>>>> > `libcamera::FrameBuffer *buf`. Unless we're going to do complicated\n>>>>>> > type diagnosis and simplification, we might end up with having five\n>>>>>> > inputs as well.\n>>>>>>\n>>>>>\nI take this back. As we'll most likely have a self-defined\nfunction that takes the two inputs with the correct C/C++\ntypes, we might still be able to maintain the input size for\nlttng macros.\n\nBR,\nHarvey\n\n\n> Also, I don't know how to indicate the usage of\n>>>>>> > ctf_integer_hex (for `req` and `buf`) and ctf_enum (for status).\n>>>>>> >\n>>>>>> > I'd still prefer the 3rd or 4th approach.\n>>>>>>\n>>>>>> The 4th option, duplicating trace points in the libcamera sources, is\n>>>>>> the one I like the least. The 3rd option is also something I don't\n>>>>>> like\n>>>>>> much. It means that anyone adding trace points will need to do extra\n>>>>>> work to support two trace systems. Testing it locally will be hard,\n>>>>>> even\n>>>>>> for build testing. We could possibly handle the build testing with CI\n>>>>>> (see below), even if that will be way less convenient that doing local\n>>>>>> builds. Runtime testing is likely out of question.\n>>>>>>\n>>>>>> I'm not comfortable with the amount of extra work this will cause to\n>>>>>> everybody to support a Google-only trace system. I'm willing to find a\n>>>>>> compromise though, if Google could do the necessary work needed to\n>>>>>> package Perfetto in distributions, I could accept extra work on our\n>>>>>> side\n>>>>>> too.\n>>>>>>\n>>>>>> > > > 2. Creating a new language from which we generate both Perfetto\n>>>>>> > > > and lttng macros.\n>>>>>> > > > Pros: Easy to update libcamera tracepoint macros.\n>>>>>> > > > Cons: Probably an overkill; hard to cover all features from\n>>>>>> both\n>>>>>> > > > Perfetto and lttng.\n>>>>>> > > >\n>>>>>> > > > 3. As this patch implements: have duplicated libcamera\n>>>>>> tracepoint\n>>>>>> > > > definitions.\n>>>>>> > > > Pros: Easy to add libcamera tracepoint macros to cover all\n>>>>>> features\n>>>>>> > > > from Perfetto & lttng.\n>>>>>> > > > Cons: Developer needs to update both sides when the\n>>>>>> definition of\n>>>>>> > > > libcamera tracepoints changes; Unused macro definitions might be\n>>>>>> > > > adopted in both sides, if a feature is only available in one\n>>>>>> trace\n>>>>>> > > library.\n>>>>>> > > >\n>>>>>> > > > 4. Add another set of libcamera tracepoints for Perfetto, or\n>>>>>> just use\n>>>>>> > > > Perfetto macros directly in libcamera's source code.\n>>>>>> > > > Pros: Two isolated trace stacks that don't influence each\n>>>>>> other. All\n>>>>>> > > > features can be implemented easily.\n>>>>>> > > > Cons: Duplicated trace code in source code that might lead to\n>>>>>> > > > confusion; developers also need to update both macros when a\n>>>>>> > > > new trace is needed.\n>>>>>> > > >\n>>>>>> > > > I think approach 3rd and 4th are easier to maintain in the long\n>>>>>> > > > term. WDYT?\n>>>>>> > > >\n>>>>>> > > > Also, I'd like to know if you think Perfetto will be used in\n>>>>>> other linux\n>>>>>> > > > distributions than ChromeOS or Android.\n>>>>>> > >\n>>>>>> > > If Perfetto gets added to the package repositories in linux\n>>>>>> > > distributions then I think it would be used :)\n>>>>>> > >\n>>>>>> > > Not being able to sudo apt install perfetto is a pretty big\n>>>>>> blocker.\n>>>>>> >\n>>>>>> > Yeah I understand, while Perfetto is self-contained and simple to\n>>>>>> build\n>>>>>> > in linux distros.\n>>>>>>\n>>>>>> Do you mean by running the tools/install-build-deps script that\n>>>>>> downloads toolchain binaries from google servers ? That doesn't really\n>>>>>> match the philosophy of Linux distributions.\n>>>>>>\n>>>>>\n>>>>> Yeah not accepting this makes sense I'm sympathetic to this.\n>>>>>\n>>>>>\n>>>>>>\n>>>>>> > May we consider testing it on the gitlab CI, after we\n>>>>>> > somehow add the support?\n>>>>>>\n>>>>>> If we add support for it I certainly would like to see it being tested\n>>>>>> in CI, yes. After a quick look at the Perfetto build system, it looks\n>>>>>> like adding Perfetto to the CI docker containers may not be trivial.\n>>>>>> Someone will need to volunteer.\n>>>>>>\n>>>>>> > > > Chinglin, please help amend anything I missed as well :)\n>>>>>>\n>>>>>> --\n>>>>>> Regards,\n>>>>>>\n>>>>>> Laurent Pinchart\n>>>>>>\n>>>>>>\n>>>>>> --\n>>>>>> BR,\n>>>>>> Harvey Yang\n>>>>>>\n>>>>>> --\n>>>>>> You received this message because you are subscribed to the Google\n>>>>>> Groups \"Perfetto-dev\" group.\n>>>>>> To unsubscribe from this group and stop receiving emails from it,\n>>>>>> send an email to perfetto-dev+unsubscribe@google.com.\n>>>>>> To view this discussion on the web visit\n>>>>>> https://groups.google.com/a/google.com/d/msgid/perfetto-dev/CAC%3DwSGWxc4W9nZOoieTMhG9h-Kh%2BwH%3D%3DiPfNwxqd0ybNntVFWQ%40mail.gmail.com\n>>>>>> \n>>>>>> .\n>>>>>>\n>>>>>\n>>\n>> --\n>> BR,\n>> Harvey Yang\n>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E8C1FC324C\n\tfor ;\n\tWed, 18 Sep 2024 07:57:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2CE3634F9;\n\tWed, 18 Sep 2024 09:57:20 +0200 (CEST)","from mail-lj1-x230.google.com (mail-lj1-x230.google.com\n\t[IPv6:2a00:1450:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 58E9A618E0\n\tfor ;\n\tWed, 18 Sep 2024 09:57:19 +0200 (CEST)","by mail-lj1-x230.google.com with SMTP id\n\t38308e7fff4ca-2f754d4a6e4so69891591fa.3\n\tfor ;\n\tWed, 18 Sep 2024 00:57:19 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"PD7DCiJw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1726646238; x=1727251038;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=Dz7oxOeYSXaEwa+kUzb57sLgOHjGdZQ/W/p1pRrzgUk=;\n\tb=PD7DCiJwPPZlyoNFd1SwfKMqONbq/1/RMvUPdxYaldjAM8/Vxvz60NamfRaTFGg/VG\n\t0vRPxa4YHr65KsAnsv2WXt+sNjMNk03BM+FP8nLCLnHWvygfL0QwAwspvSYjiY45vint\n\tkUjMarTZNpsZri7/FzaNAgBRNM/xecmCbjnP8=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1726646238; x=1727251038;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=Dz7oxOeYSXaEwa+kUzb57sLgOHjGdZQ/W/p1pRrzgUk=;\n\tb=HEw5IMOsE1f31ejWy9SVwooWzeQlATsIFl7E2URvM0XrivuFlUAqH2ezJ4izLkVfXv\n\tOhvlR9Q+5TN+feQLfqqe2YTFKv1zva6UzF5QSmcgeugB4EmCbXuGmuCGbYGvhrFh8i6X\n\txcNJScILbhpsBRfOmcYaYH/fw144J6JK8g7sg4+OehX3SGVNSSYWpXJXgoJb7zGAJoLf\n\tMJ7VLPHeugYyB9rF1qi9nilG2gbx59i7540UfbcOP76TO9Uck93HuSbgXhplbCvQG51f\n\t4xgI/7UkdgCjrRIPwANTQEC/RPayZaXOt5qPVtm9de2zwMMfVef1K1Pwx4aGuF6bRtDX\n\tHDmw==","X-Gm-Message-State":"AOJu0YwL9Zg2zerNw8aDolpakTBCcBAWtiX+uuZAl1V4zkSaqx0IrJSA\n\teoyJcdOQt3I8JE3QtvHHCV4HcXW/8KU4kRraOG7N5TX8MCgl88us42+G030DEpHcz1yiy4/DJ2v\n\ti5stQ+VA3uhKoxkRjk6NjYY3cQ6OT/07IgcxvEVxIpDpHDJU=","X-Google-Smtp-Source":"AGHT+IHE3mdR9zZOVVHiCh3UtUWSSXyZonFkVgpT7JDjCUIK1Et3azPn8ZjwAFNq8C9iLElvwiqIFSPh/ikjGNNkQQo=","X-Received":"by 2002:a2e:4a19:0:b0:2f3:f8d7:d556 with SMTP id\n\t38308e7fff4ca-2f787dc864dmr85686961fa.18.1726646237654;\n\tWed, 18 Sep 2024 00:57:17 -0700 (PDT)","MIME-Version":"1.0","References":"<20221220070523.3212844-1-chenghaoyang@google.com>\n\t<20221220070523.3212844-2-chenghaoyang@google.com>\n\t\n\t\n\t\n\t\n\t<20240911111011.GC4470@pendragon.ideasonboard.com>\n\t\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Wed, 18 Sep 2024 15:57:06 +0800","Message-ID":"","Subject":"Re: [libcamera-devel] [PATCH v3 1/1] Use tracing with perfetto in\n\tChromeOS","To":"Laurent Pinchart via libcamera-devel\n\t, \n\tLaurent Pinchart , \n\tPaul Elder ","Cc":"Cheng-Hao Yang ,\n\tDaniele Di Proietto , \n\tPrimiano Tucci ,\n\tPerfetto-dev , \n\tTomasz Figa , Han-lin Chen , \n\tLalit Maganti ,\n\tChinglin Yu ","Content-Type":"multipart/alternative; boundary=\"000000000000a60c89062260274e\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]