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. The idea of the usage in SPICE is to collect data while the program run. Using current SPICE logging facility was discussed but not easy to filter data. Other solutions (SystemTap, LTTng) were discarded due to not cross platform. A printf based solution was discussed too but missing the additional tools which are useful. Currently we don't plan to use as extensively as to be a problem to be replaced or removed in the future. Both Autoconf and Meson build systems 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@xxxxxxxxxx> --- .gitmodules | 3 ++ common/Makefile.am | 10 +++++ common/log.c | 3 ++ common/meson.build | 12 +++++- common/recorder | 1 + common/recorder.h | 74 +++++++++++++++++++++++++++++++++++++ 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, 205 insertions(+), 4 deletions(-) create mode 160000 common/recorder create mode 100644 common/recorder.h create mode 100644 tests/test-dummy-recorder.c Changes since v1: - rebased on master; - added some comments; - added example/usage for spice-server. 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 197e419..3318009 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -53,8 +53,18 @@ libspice_common_la_SOURCES = \ utils.c \ utils.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 8f22310..156297b 100644 --- a/common/meson.build +++ b/common/meson.build @@ -37,9 +37,19 @@ spice_common_sources = [ 'snd_codec.h', 'utils.c', 'utils.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..98b8797 --- /dev/null +++ b/common/recorder.h @@ -0,0 +1,74 @@ +/* + 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> +#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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel