[libcamera-devel] [PATCH v3 6/6] Documentation: tracing: Add tracing guide

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Thu, Oct 29, 2020 at 07:16:29PM +0900, Paul Elder wrote:
> Add guide for tracepoints, including how to define and use them.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v3:
> - fix compilation error
> - reword english
> - add stuff about the new IPA call tracepoint macros
> 
> New in v2
> ---
>  Documentation/guides/tracing.rst | 140 +++++++++++++++++++++++++++++++
>  Documentation/index.rst          |   1 +
>  Documentation/meson.build        |   1 +
>  3 files changed, 142 insertions(+)
>  create mode 100644 Documentation/guides/tracing.rst
> 
> diff --git a/Documentation/guides/tracing.rst b/Documentation/guides/tracing.rst
> new file mode 100644
> index 00000000..8c59e7ed
> --- /dev/null
> +++ b/Documentation/guides/tracing.rst
> @@ -0,0 +1,140 @@
> +.. SPDX-License-Identifier: CC-BY-SA-4.0
> +
> +Tracing Guide
> +=============
> +
> +Guide to tracing in libcamera.
> +
> +Profiling vs Tracing
> +--------------------
> +
> +Tracing is recording timestamps at specific locations. libcamera provides a
> +tracing facility. This guide shows how to use this tracing facility.
> +
> +Tracing should not be confused with profiling, which samples execution
> +at periodic points in time. This can be done with other tools such as
> +callgrind, perf, gprof, etc., without modification to the application,
> +and is out of scope for this guide.
> +
> +Compiling
> +---------
> +
> +To compile libcamera with tracing support, it must be enabled through the
> +meson ``tracing`` option. It depends on the lttng-ust library (available in the
> +``liblttng-ust-dev`` package for Debian-based distributions).
> +By default the tracing option in meson is set to ``auto``, so if
> +liblttng is detected, it will be enabled by default. Conversely, if the option
> +is set to disabled, then libcamera will be compiled without tracing support.
> +
> +Defining tracepoints
> +--------------------
> +
> +The first of two steps to using tracepoints is to define the tracepoints.
> +

How about adding here

"libcamera already contains a set of tracepoints. To define additional
tracepoints, create ..."

?

> +Create a file ``include/libcamera/internal/tracepoints/{file}.tp``, where
> +``file`` is a reasonable name related to the category of tracepoints that
> +you wish to define. For example, a tracepoints file for the Request object

s/a tracepoints/the tracepoints/

> +would be called ``request.tp``. An entry for this file must be added in

s/would be/is/

as it already exists.

> +``include/libcamera/internal/tracepoints/meson.build``.
> +
> +In this tracepoints file, define your tracepoints `as mandated by lttng
> +<https://lttng.org/man/3/lttng-ust>`_. The header boilerplate must *not* be
> +included (as it will conflict with the rest of our infrastructure), and
> +only the tracepoint definitions (with the ``TRACEPOINT_*`` macros) should be
> +included.
> +
> +All tracepoint providers shall be ``libcamera``. According to lttng, the
> +tracepoint provider should be per-project; this is the rationale for this
> +decision. To group tracepoint events, we recommend using
> +``{class_name}_{tracepoint_name}``, for example, ``request_construct`` for a
> +tracepoint for the constructor of the Request class.
> +
> +Tracepoint arguments may take C++ objects pointers, in which case the usual
> +C++ namespacing rules apply. The header that contains the necessary class
> +definitions must be included at the top of the tracepoint provider file.
> +
> +Note: the final parameter in ``TP_ARGS`` *must not* have a trailing comma, and
> +the parameters to ``TP_FIELDS`` are *space-separated*. Not following these will
> +cause compilation errors.
> +
> +Using tracepoints (in libcamera)
> +--------------------------------
> +
> +To use tracepoints in libcamera, first the header needs to be included:
> +
> +``#include "libcamera/internal/tracepoints.h"``
> +
> +Then to use the tracepoint:
> +
> +``LIBCAMERA_TRACEPOINT({tracepoint_event}, args...)``
> +
> +This macro must be used, as opposed to lttng's macros directly, because
> +lttng is an optional dependency of libcamera, so the code must compile and run
> +even when lttng is not present or when tracing is disabled.
> +
> +The tracepoint provider name, as declared in the tracepoint definition, is not
> +included in the parameters of the tracepoint.
> +
> +There are also two special tracepoints available for tracing IPA calls:
> +
> +``LIBCAMERA_TRACEPOINT_IPA_BEGIN({pipeline_name}, {ipa_function})``
> +
> +``LIBCAMERA_TRACEPOINT_IPA_END({pipeline_name}, {ipa_function})``
> +
> +These shall be placed where an IPA function is called from the pipeline handler,
> +and when the pipeline handler receives the corresponding response from the IPA,
> +respecively. These are the tracepoints that our sample analysis script

s/respecively/respectively/

> +(see "Analyzing a trace") scans for when computing statistics on IPA call time.
> +
> +Using tracepoints (from an application)
> +---------------------------------------
> +
> +As applications are not part of libcamera, but rather users of libcamera,
> +applications should seek their own tracing mechanisms. For ease of tracing
> +the application alongside tracing libcamera, it is recommended to also
> +`use lttng <https://lttng.org/docs/#doc-tracing-your-own-user-application>`_.
> +
> +Using tracepoints (from closed-source IPA)
> +------------------------------------------
> +
> +Similar to applications, closed-source IPAs can simply use lttng on their own,
> +or any other tracing mechanism if desired.
> +
> +Collecting a trace
> +------------------
> +
> +A trace can be collected fairly simply from lttng:
> +
> +.. code-block:: bash
> +
> +   lttng create $SESSION_NAME
> +   lttng enable-event -u libcamera:\*
> +   lttng start
> +   # run libcamera application
> +   lttng stop
> +   lttng view
> +   lttng destroy $SESSION_NAME
> +
> +See the `lttng documentation <https://lttng.org/docs/>`_ for further details.
> +
> +Analyzing a trace
> +-----------------
> +
> +As mentioned above, while an lttng tracing session exists and the trace is not
> +running, the trace output can be viewd as text by ``lttng view``.

s/viewd/viewed/

> +
> +The trace log can also be viewed as text using babeltrace2.  See the
> +`lttng trace analysis documentation <https://lttng.org/docs/#doc-viewing-and-analyzing-your-traces-bt>`_
> +for further details.
> +
> +babeltrace2 also has a C API and python bindings that can be used to process
> +traces. See the
> +`lttng python bindings documentation <https://babeltrace.org/docs/v2.0/python/bt2/>`_

Can't you break lines in the middle of the link name to make the reflow
a bit nicer ? It's one once above so I assume it should work.

> +and the
> +`lttng C API documentation <https://babeltrace.org/docs/v2.0/libbabeltrace2/>`_
> +for more details.
> +
> +As an example, there is a script ``utils/tracepoints/analyze.py`` that gathers

Don't forget to update the script name here if you rename it as proposed
in the review of patch 5/6.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +statistics for the time taken for an IPA function call, by measuring the time
> +difference between pairs of events ``libcamera:ipa_call_start`` and
> +``libcamera:ipa_call_finish``.
> diff --git a/Documentation/index.rst b/Documentation/index.rst
> index 173c326f..8bc8922e 100644
> --- a/Documentation/index.rst
> +++ b/Documentation/index.rst
> @@ -17,3 +17,4 @@
>     Application Writer's Guide <guides/application-developer>
>     Pipeline Handler Writer's Guide <guides/pipeline-handler>
>     IPA Writer's guide <guides/ipa>
> +   Tracing guide <guides/tracing>
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index f2300dac..17f3b9d7 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -55,6 +55,7 @@ if sphinx.found()
>          'guides/ipa.rst',
>          'guides/application-developer.rst',
>          'guides/pipeline-handler.rst',
> +        'guides/tracing.rst',
>      ]
>  
>      release = 'release=v' + libcamera_git_version

-- 
Regards,

Laurent Pinchart