[Spice-devel] [PATCH spice-common] Integrate recorder library

Frediano Ziglio fziglio at redhat.com
Mon Nov 19 20:46:04 UTC 2018


From: Christophe de Dinechin <dinechin at redhat.com>

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