[Spice-devel] [PATCH spice-common] Integrate recorder library
Frediano Ziglio
fziglio at redhat.com
Mon Nov 19 20:48:02 UTC 2018
>
> From: Christophe de Dinechin <dinechin at redhat.com>
>
Well... probably I started with some other patches and git kept the author.
> Allow to use recorder library. See https://github.com/c3d/recorder for
> details.
> The main usage will be to collect statistics while the programs will run.
> By default the recorder will be disabled at compile time.
> Both autoconf and Meson are supported.
> Autoconf requires the addition of SPICE_CHECK_RECORDER call in configure.ac.
> Meson requires to add recorder option.
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> .gitmodules | 3 ++
> common/Makefile.am | 10 +++++
> common/log.c | 3 ++
> common/meson.build | 12 +++++-
> common/recorder | 1 +
> common/recorder.h | 75 +++++++++++++++++++++++++++++++++++++
> configure.ac | 8 +++-
> m4/spice-deps.m4 | 15 ++++++++
> meson.build | 12 +++++-
> meson_options.txt | 6 +++
> tests/Makefile.am | 12 ++++++
> tests/test-dummy-recorder.c | 53 ++++++++++++++++++++++++++
> 12 files changed, 206 insertions(+), 4 deletions(-)
> create mode 160000 common/recorder
> create mode 100644 common/recorder.h
> create mode 100644 tests/test-dummy-recorder.c
>
> diff --git a/.gitmodules b/.gitmodules
> index e69de29..a7ea8b1 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -0,0 +1,3 @@
> +[submodule "common/recorder"]
> + path = common/recorder
> + url = https://github.com/c3d/recorder.git
> diff --git a/common/Makefile.am b/common/Makefile.am
> index da0d941..4c0a6cd 100644
> --- a/common/Makefile.am
> +++ b/common/Makefile.am
> @@ -51,8 +51,18 @@ libspice_common_la_SOURCES = \
> snd_codec.c \
> snd_codec.h \
> verify.h \
> + recorder.h \
> $(NULL)
>
> +if ENABLE_RECORDER
> +libspice_common_la_SOURCES += \
> + recorder/recorder.c \
> + recorder/recorder.h \
> + recorder/recorder_ring.c \
> + recorder/recorder_ring.h \
> + $(NULL)
> +endif
> +
> # These 2 files are not build as part of spice-common
> # build system, but modules using spice-common will build
> # them with the appropriate options. We need to let automake
> diff --git a/common/log.c b/common/log.c
> index b59c8a8..a7806c5 100644
> --- a/common/log.c
> +++ b/common/log.c
> @@ -27,6 +27,8 @@
> #include <unistd.h>
> #endif
>
> +#include <common/recorder.h>
> +
> #include "log.h"
> #include "backtrace.h"
>
> @@ -154,6 +156,7 @@ SPICE_CONSTRUCTOR_FUNC(spice_log_init)
> if (!g_thread_supported())
> g_thread_init(NULL);
> #endif
> + recorder_dump_on_common_signals(0, 0);
> }
>
> static void spice_logv(const char *log_domain,
> diff --git a/common/meson.build b/common/meson.build
> index 6ac55dc..e715e31 100644
> --- a/common/meson.build
> +++ b/common/meson.build
> @@ -35,9 +35,19 @@ spice_common_sources = [
> 'rop3.h',
> 'snd_codec.c',
> 'snd_codec.h',
> - 'verify.h'
> + 'verify.h',
> + 'recorder.h'
> ]
>
> +if get_option('recorder')
> + spice_common_sources += [
> + 'recorder/recorder.c',
> + 'recorder/recorder.h',
> + 'recorder/recorder_ring.c',
> + 'recorder/recorder_ring.h'
> + ]
> +endif
> +
> spice_common_lib = static_library('spice-common', spice_common_sources,
> install : false,
> include_directories :
> spice_common_include,
> diff --git a/common/recorder b/common/recorder
> new file mode 160000
> index 0000000..8f450dc
> --- /dev/null
> +++ b/common/recorder
> @@ -0,0 +1 @@
> +Subproject commit 8f450dcf0ec725a41b80c495ecdd6110dd1fcdb8
> diff --git a/common/recorder.h b/common/recorder.h
> new file mode 100644
> index 0000000..4496de9
> --- /dev/null
> +++ b/common/recorder.h
> @@ -0,0 +1,75 @@
> +/*
> + Copyright (C) 2018 Red Hat, Inc.
> +
> + This library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + This library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with this library; if not, see
> <http://www.gnu.org/licenses/>.
> +*/
> +/* This file include recorder library headers or if disabled provide
> + * replacement declarations */
> +#ifndef ENABLE_RECORDER
> +
> +#include <stdio.h>
> +#include <stdint.h>
> +
> +/* Replacement declarations.
> + * There declarations should generate no code (beside when no optimization
> are
> + * selected) but catch some possible programming warnings/errors at
> + * compile/link time like:
> + * - usage of undeclared recorders;
> + * - recording formats and arguments;
> + * - matching RECORD_TIMING_BEGIN and RECORD_TIMING_END.
> + * The only exceptions are tweaks which just generate a variable to hold the
> + * value.
> + */
> +
> +typedef struct SpiceEmptyStruct {
> + char dummy[0];
> +} SpiceEmptyStruct;
> +
> +typedef struct SpiceDummyTweak {
> + intptr_t tweak_value;
> +} SpiceDummyTweak;
> +
> +#define RECORDER_DECLARE(rec) \
> + extern const SpiceEmptyStruct spice_recorder_ ## rec
> +#define RECORDER(rec, num_rings, comment) \
> + RECORDER_DEFINE(rec, num_rings, comment)
> +#define RECORDER_DEFINE(rec, num_rings, comment) \
> + const SpiceEmptyStruct spice_recorder_ ## rec = {}
> +#define RECORDER_TRACE(rec) \
> + (sizeof(spice_recorder_ ## rec) != sizeof(SpiceEmptyStruct))
> +#define RECORDER_TWEAK_DECLARE(rec) \
> + extern const SpiceDummyTweak spice_recorder_tweak_ ## rec
> +#define RECORDER_TWEAK_DEFINE(rec, value, comment) \
> + const SpiceDummyTweak spice_recorder_tweak_ ## rec = { (value) }
> +#define RECORDER_TWEAK(rec) \
> + ((spice_recorder_tweak_ ## rec).tweak_value)
> +#define RECORD(rec, format, ...) do { \
> + if (sizeof((spice_recorder_ ## rec).dummy)) printf(format,
> ##__VA_ARGS__); \
> + } while(0)
> +#define RECORD_TIMING_BEGIN(rec) \
> + do { RECORD(rec, "begin");
> +#define RECORD_TIMING_END(rec, op, name, value) \
> + RECORD(rec, "end" op name); \
> + } while(0)
> +#define record(...) \
> + RECORD(__VA_ARGS__)
> +static inline void
> +recorder_dump_on_common_signals(unsigned add, unsigned remove)
> +{
> +}
> +
> +#else
> +#include <common/recorder/recorder.h>
> +#include <common/recorder/recorder_ring.h>
> +#endif
> diff --git a/configure.ac b/configure.ac
> index 6e1f5a8..923055b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,7 +13,7 @@ AC_CONFIG_AUX_DIR([build-aux])
> m4_ifdef([AM_PROG_AR], [AM_PROG_AR])
>
> # Checks for programs
> -AM_INIT_AUTOMAKE([1.11 dist-xz no-dist-gzip tar-ustar foreign -Wall
> -Werror])
> +AM_INIT_AUTOMAKE([1.11 dist-xz no-dist-gzip tar-ustar foreign subdir-objects
> -Wall -Werror])
> AM_MAINTAINER_MODE
> AM_SILENT_RULES([yes])
> LT_INIT
> @@ -26,6 +26,10 @@ if test "x$ac_cv_prog_cc_c99" = xno; then
> fi
> AM_PROG_CC_C_O
>
> +AC_CHECK_HEADERS([sys/mman.h regex.h])
> +AC_CHECK_FUNCS([sigaction drand48])
> +AC_SEARCH_LIBS(regcomp, [regex rx])
> +
> SPICE_CHECK_SYSDEPS
> SPICE_EXTRA_CHECKS
>
> @@ -37,6 +41,8 @@ AC_ARG_ENABLE([alignment-checks],
> AS_IF([test "x$enable_alignment_checks" = "xyes"],
> [AC_DEFINE([SPICE_DEBUG_ALIGNMENT], 1, [Enable runtime checks for cast
> alignment])])
>
> +SPICE_CHECK_RECORDER
> +
> # Checks for libraries
> PKG_CHECK_MODULES([PROTOCOL], [spice-protocol >= 0.12.12])
>
> diff --git a/m4/spice-deps.m4 b/m4/spice-deps.m4
> index 1721d34..4bf4eb5 100644
> --- a/m4/spice-deps.m4
> +++ b/m4/spice-deps.m4
> @@ -363,3 +363,18 @@ AC_DEFUN([SPICE_CHECK_SASL], [
> AC_DEFUN([SPICE_CHECK_OPENSSL], [
> PKG_CHECK_MODULES(OPENSSL, openssl)
> ])
> +
> +# SPICE_CHECK_RECORDER
> +# -----------------
> +# Check for the availability of recorder library.
> +#------------------
> +AC_DEFUN([SPICE_CHECK_RECORDER], [
> + AC_ARG_ENABLE([recorder],
> + AS_HELP_STRING([--enable-recorder],
> + [Enable recorder instrumentation @<:@default=no@:>@]),
> + [],
> + enable_recorder="no")
> + AS_IF([test "$enable_recorder" = "yes"],
> + AC_DEFINE([ENABLE_RECORDER], [1], [Define if recorder
> instrumentation is enabled]))
> + AM_CONDITIONAL([ENABLE_RECORDER],[test "$enable_recorder" = "yes"])
> +])
> diff --git a/meson.build b/meson.build
> index 049409b..f6ae0f2 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -34,6 +34,10 @@ if get_option('extra-checks')
> spice_common_config_data.set('ENABLE_EXTRA_CHECKS', '1')
> endif
>
> +if get_option('recorder')
> + spice_common_config_data.set('ENABLE_RECORDER', '1')
> +endif
> +
> spice_common_generate_code = get_option('generate-code')
> spice_common_generate_client_code = spice_common_generate_code == 'all' or
> spice_common_generate_code == 'client'
> spice_common_generate_server_code = spice_common_generate_code == 'all' or
> spice_common_generate_code == 'server'
> @@ -56,7 +60,9 @@ headers = ['alloca.h',
> 'sys/socket.h',
> 'sys/stat.h',
> 'sys/types.h',
> - 'unistd.h']
> + 'unistd.h',
> + 'regex.h',
> + 'sys/mman.h']
>
> foreach header : headers
> if compiler.has_header(header)
> @@ -72,7 +78,9 @@ functions = ['alloca',
> 'floor',
> 'fork',
> 'memmove',
> - 'memset']
> + 'memset',
> + 'sigaction',
> + 'drand48']
>
> foreach func : functions
> if compiler.has_function(func)
> diff --git a/meson_options.txt b/meson_options.txt
> index 1b80257..a90726a 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -20,6 +20,12 @@ option('opus',
> yield : true,
> description: 'Enable Opus audio codec')
>
> +option('recorder',
> + type : 'boolean',
> + value : false,
> + yield : true,
> + description: 'Enable recorder instrumentation')
> +
> option('smartcard',
> type : 'boolean',
> value : true,
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 926ac99..690972f 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -65,6 +65,18 @@ test_region_LDADD = \
> $(top_builddir)/common/libspice-common.la \
> $(NULL)
>
> +TESTS += test_dummy_recorder
> +
> +test_dummy_recorder_SOURCES = \
> + test-dummy-recorder.c \
> + $(NULL)
> +test_dummy_recorder_CFLAGS = \
> + -I$(top_srcdir) \
> + $(NULL)
> +test_dummy_recorder_LDADD = \
> + $(top_builddir)/common/libspice-common.la \
> + $(NULL)
> +
> # Avoid need for python(pyparsing) by end users
> TEST_MARSHALLERS = \
> generated_test_marshallers.c \
> diff --git a/tests/test-dummy-recorder.c b/tests/test-dummy-recorder.c
> new file mode 100644
> index 0000000..4e674a9
> --- /dev/null
> +++ b/tests/test-dummy-recorder.c
> @@ -0,0 +1,53 @@
> +/*
> + Copyright (C) 2018 Red Hat, Inc.
> +
> + This library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + This library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with this library; if not, see
> <http://www.gnu.org/licenses/>.
> +*/
> +/* Test that the macros to replace recorder works correctly */
> +#undef NDEBUG
> +#include <config.h>
> +#include <assert.h>
> +
> +#undef ENABLE_RECORDER
> +
> +#include <common/recorder.h>
> +
> +RECORDER(rec1, 32, "Desc rec1");
> +RECORDER_DECLARE(rec2);
> +
> +RECORDER_TWEAK_DECLARE(tweak);
> +
> +int main(void)
> +{
> + // this should compile and link
> + recorder_dump_on_common_signals(0, 0);
> +
> + // dummy traces should be always disabled
> + if (RECORDER_TRACE(rec1)) {
> + return 1;
> + }
> +
> + // check tweak value
> + assert(RECORDER_TWEAK(tweak) == 123);
> +
> + RECORD(rec2, "aaa %d", 1);
> +
> + RECORD_TIMING_BEGIN(rec2);
> + RECORD_TIMING_END(rec2, "op", "name", 123);
> +
> + return 0;
> +}
> +
> +RECORDER_DEFINE(rec2, 64, "Desc rec2");
> +RECORDER_TWEAK_DEFINE(tweak, 123, "Desc tweak");
> --
> 2.17.2
>
>
More information about the Spice-devel
mailing list