[Spice-commits] 2 commits - server/dcc.c server/display-channel.c server/display-channel.h server/stat.h server/tests
Frediano Ziglio
fziglio at kemper.freedesktop.org
Wed Dec 9 09:21:55 PST 2015
server/dcc.c | 31 ++++++-----------
server/display-channel.c | 47 +++++++++-----------------
server/display-channel.h | 4 --
server/stat.h | 67 +++++++++++++++++++++++++------------
server/tests/.gitignore | 5 ++
server/tests/Makefile.am | 35 +++++++++++++++++++
server/tests/stat-main.c | 49 +++++++++++++++++++++++++++
server/tests/stat-test.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++
8 files changed, 246 insertions(+), 75 deletions(-)
New commits:
commit c4c716ab18af65c4c72daeacbab6c5e6489cbe5e
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Thu Nov 12 22:08:48 2015 +0000
stat: add test for statistic functions
Make sure code compile with and without statistics enabled (beside
printing functions).
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
diff --git a/server/tests/.gitignore b/server/tests/.gitignore
index 1059247..17ffa2c 100644
--- a/server/tests/.gitignore
+++ b/server/tests/.gitignore
@@ -9,3 +9,8 @@ spice-server-replay
test_display_width_stride
test_two_servers
test_vdagent
+stat_test
+libstat_test1.a
+libstat_test2.a
+libstat_test3.a
+libstat_test4.a
diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
index b398fe4..fd7e26d 100644
--- a/server/tests/Makefile.am
+++ b/server/tests/Makefile.am
@@ -44,6 +44,20 @@ noinst_PROGRAMS = \
test_vdagent \
test_display_width_stride \
spice-server-replay \
+ stat_test \
+ $(NULL)
+
+TESTS = \
+ stat_test \
+ $(NULL)
+
+check_PROGRAMS = $(TESTS)
+
+noinst_LIBRARIES = \
+ libstat_test1.a \
+ libstat_test2.a \
+ libstat_test3.a \
+ libstat_test4.a \
$(NULL)
test_vdagent_SOURCES = \
@@ -113,3 +127,24 @@ spice_server_replay_SOURCES = \
basic_event_loop.h \
test_util.h \
$(NULL)
+
+stat_test_SOURCES = stat-main.c
+stat_test_LDADD = \
+ $(LDADD) \
+ libstat_test1.a \
+ libstat_test2.a \
+ libstat_test3.a \
+ libstat_test4.a \
+ $(NULL)
+
+libstat_test1_a_SOURCES = stat-test.c
+libstat_test1_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=0 -DTEST_RED_WORKER_STAT=0 -DTEST_NAME=stat_test1
+
+libstat_test2_a_SOURCES = stat-test.c
+libstat_test2_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=0 -DTEST_RED_WORKER_STAT=1 -DTEST_NAME=stat_test2
+
+libstat_test3_a_SOURCES = stat-test.c
+libstat_test3_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=1 -DTEST_RED_WORKER_STAT=0 -DTEST_NAME=stat_test3
+
+libstat_test4_a_SOURCES = stat-test.c
+libstat_test4_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=1 -DTEST_RED_WORKER_STAT=1 -DTEST_NAME=stat_test4
diff --git a/server/tests/stat-main.c b/server/tests/stat-main.c
new file mode 100644
index 0000000..3d2e48a
--- /dev/null
+++ b/server/tests/stat-main.c
@@ -0,0 +1,49 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ Copyright (C) 2015 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/>.
+*/
+
+/* check statistics
+ */
+
+#include <config.h>
+#include <glib.h>
+
+typedef void test_func_t(void);
+
+test_func_t stat_test1;
+test_func_t stat_test2;
+test_func_t stat_test3;
+test_func_t stat_test4;
+
+static test_func_t *test_funcs[] = {
+ stat_test1,
+ stat_test2,
+ stat_test3,
+ stat_test4,
+ NULL
+};
+
+int main(void)
+{
+ test_func_t **func_p;
+
+ for (func_p = test_funcs; *func_p; ++func_p) {
+ (*func_p)();
+ }
+ return 0;
+}
+
diff --git a/server/tests/stat-test.c b/server/tests/stat-test.c
new file mode 100644
index 0000000..b2b2136
--- /dev/null
+++ b/server/tests/stat-test.c
@@ -0,0 +1,83 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ Copyright (C) 2015 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/>.
+*/
+
+/* check statistics
+ */
+
+#include <config.h>
+
+#undef COMPRESS_STAT
+#undef RED_WORKER_STAT
+
+#if TEST_COMPRESS_STAT
+#define COMPRESS_STAT
+#endif
+
+#if TEST_RED_WORKER_STAT
+#define RED_WORKER_STAT
+#endif
+
+#include <unistd.h>
+#include <glib.h>
+#include "../stat.h"
+
+#ifndef TEST_NAME
+#error TEST_NAME must be defined!
+#endif
+
+void TEST_NAME(void)
+{
+ stat_info_t info;
+ stat_start_time_t start_time;
+
+#if !defined(COMPRESS_STAT) && !defined(RED_WORKER_STAT)
+ G_STATIC_ASSERT(sizeof(start_time) == 0);
+ G_STATIC_ASSERT(sizeof(info) == 0);
+#endif
+
+ stat_init(&info, "test", CLOCK_MONOTONIC);
+ stat_start_time_init(&start_time, &info);
+ usleep(2);
+ stat_add(&info, start_time);
+
+#ifdef RED_WORKER_STAT
+ g_assert_cmpuint(info.count, ==, 1);
+ g_assert_cmpuint(info.min, ==, info.max);
+ g_assert_cmpuint(info.min, >=, 2000);
+ g_assert_cmpuint(info.min, <, 100000000);
+#endif
+
+ stat_reset(&info);
+
+ stat_compress_init(&info, "test", CLOCK_MONOTONIC);
+ stat_start_time_init(&start_time, &info);
+ usleep(2);
+ stat_compress_add(&info, start_time, 100, 50);
+ usleep(1);
+ stat_compress_add(&info, start_time, 1000, 500);
+
+#ifdef COMPRESS_STAT
+ g_assert_cmpuint(info.count, ==, 2);
+ g_assert_cmpuint(info.min, !=, info.max);
+ g_assert_cmpuint(info.min, >=, 2000);
+ g_assert_cmpuint(info.min, <, 100000000);
+ g_assert_cmpuint(info.total, >=, 5000);
+ g_assert_cmpuint(info.orig_size, ==, 1100);
+ g_assert_cmpuint(info.comp_size, ==, 550);
+#endif
+}
commit 253d2cef40caa4c21679efd528dfa5dcaa8e3050
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Wed Dec 2 11:31:29 2015 +0000
stat: use a better design for statistic functions
make sure code compile with statistics enabled or disabled.
Dummy (empty) structures and functions are used instead of preprocessor.
Also fix a problem as stat_compress_init did not initialize clock
field.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
diff --git a/server/dcc.c b/server/dcc.c
index 78452f4..5d55e5e 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -651,9 +651,8 @@ int dcc_compress_image_glz(DisplayChannelClient *dcc,
compress_send_data_t* o_comp_data)
{
DisplayChannel *display_channel = DCC_TO_DC(dcc);
-#ifdef COMPRESS_STAT
- stat_time_t start_time = stat_now(display_channel->glz_stat.clock);
-#endif
+ stat_start_time_t start_time;
+ stat_start_time_init(&start_time, &display_channel->zlib_glz_stat);
spice_assert(bitmap_fmt_is_rgb(src->format));
GlzData *glz_data = &dcc->glz_data;
ZlibData *zlib_data;
@@ -687,9 +686,7 @@ int dcc_compress_image_glz(DisplayChannelClient *dcc,
if (!display_channel->enable_zlib_glz_wrap || (glz_size < MIN_GLZ_SIZE_FOR_ZLIB)) {
goto glz;
}
-#ifdef COMPRESS_STAT
- start_time = stat_now(display_channel->zlib_glz_stat.clock);
-#endif
+ stat_start_time_init(&start_time, &display_channel->zlib_glz_stat);
zlib_data = &dcc->zlib_data;
zlib_data->data.bufs_tail = compress_buf_new();
@@ -741,9 +738,8 @@ int dcc_compress_image_lz(DisplayChannelClient *dcc,
LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
int size; // size of the compressed data
-#ifdef COMPRESS_STAT
- stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->lz_stat.clock);
-#endif
+ stat_start_time_t start_time;
+ stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->lz_stat);
lz_data->data.bufs_tail = compress_buf_new();
lz_data->data.bufs_head = lz_data->data.bufs_tail;
@@ -817,10 +813,9 @@ int dcc_compress_image_jpeg(DisplayChannelClient *dcc, SpiceImage *dest,
int comp_head_left;
int stride;
uint8_t *lz_out_start_byte;
+ stat_start_time_t start_time;
+ stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->jpeg_alpha_stat);
-#ifdef COMPRESS_STAT
- stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->jpeg_stat.clock);
-#endif
switch (src->format) {
case SPICE_BITMAP_FMT_16BIT:
jpeg_in_type = JPEG_IMAGE_TYPE_RGB16;
@@ -940,10 +935,8 @@ int dcc_compress_image_lz4(DisplayChannelClient *dcc, SpiceImage *dest,
Lz4Data *lz4_data = &dcc->lz4_data;
Lz4EncoderContext *lz4 = dcc->lz4;
int lz4_size = 0;
-
-#ifdef COMPRESS_STAT
- stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->lz4_stat.clock);
-#endif
+ stat_start_time_t start_time;
+ stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->lz4_stat);
lz4_data->data.bufs_tail = compress_buf_new();
lz4_data->data.bufs_head = lz4_data->data.bufs_tail;
@@ -1003,10 +996,8 @@ int dcc_compress_image_quic(DisplayChannelClient *dcc, SpiceImage *dest,
QuicContext *quic = dcc->quic;
volatile QuicImageType type;
int size, stride;
-
-#ifdef COMPRESS_STAT
- stat_time_t start_time = stat_now(DCC_TO_DC(dcc)->quic_stat.clock);
-#endif
+ stat_start_time_t start_time;
+ stat_start_time_init(&start_time, &DCC_TO_DC(dcc)->quic_stat);
switch (src->format) {
case SPICE_BITMAP_FMT_32BIT:
diff --git a/server/display-channel.c b/server/display-channel.c
index 32d87af..1f9a0ed 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -29,26 +29,13 @@ uint32_t display_channel_generate_uid(DisplayChannel *display)
return ++display->bits_unique;
}
-static stat_time_t display_channel_stat_now(DisplayChannel *display)
-{
-#ifdef RED_WORKER_STAT
- RedWorker *worker = COMMON_CHANNEL(display)->worker;
-
- return stat_now(red_worker_get_clockid(worker));
-
-#else
- return 0;
-#endif
-}
-
-#define stat_start(display, var) \
- G_GNUC_UNUSED stat_time_t var = display_channel_stat_now((display));
+#define stat_start(stat, var) \
+ stat_start_time_t var; stat_start_time_init(&var, stat);
void display_channel_compress_stats_reset(DisplayChannel *display)
{
spice_return_if_fail(display);
-#ifdef COMPRESS_STAT
stat_reset(&display->quic_stat);
stat_reset(&display->lz_stat);
stat_reset(&display->glz_stat);
@@ -56,7 +43,6 @@ void display_channel_compress_stats_reset(DisplayChannel *display)
stat_reset(&display->zlib_glz_stat);
stat_reset(&display->jpeg_alpha_stat);
stat_reset(&display->lz4_stat);
-#endif
}
void display_channel_compress_stats_print(const DisplayChannel *display_channel)
@@ -568,7 +554,7 @@ static void __exclude_region(DisplayChannel *display, Ring *ring, TreeItem *item
Ring **top_ring, Drawable *frame_candidate)
{
QRegion and_rgn;
- stat_start(display, start_time);
+ stat_start(&display->__exclude_stat, start_time);
region_clone(&and_rgn, rgn);
region_and(&and_rgn, &item->rgn);
@@ -637,7 +623,7 @@ static void exclude_region(DisplayChannel *display, Ring *ring, RingItem *ring_i
QRegion *rgn, TreeItem **last, Drawable *frame_candidate)
{
Ring *top_ring;
- stat_start(display, start_time);
+ stat_start(&display->exclude_stat, start_time);
if (!ring_item) {
return;
@@ -692,7 +678,7 @@ static void exclude_region(DisplayChannel *display, Ring *ring, RingItem *ring_i
static int current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawable *item)
{
- stat_start(display, start_time);
+ stat_start(&display->add_stat, start_time);
#ifdef RED_WORKER_STAT
++display->add_with_shadow_count;
#endif
@@ -739,7 +725,7 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable)
RingItem *now;
QRegion exclude_rgn;
RingItem *exclude_base = NULL;
- stat_start(display, start_time);
+ stat_start(&display->add_stat, start_time);
spice_assert(!region_is_empty(&item->base.rgn));
region_init(&exclude_rgn);
@@ -2051,9 +2037,10 @@ DisplayChannel* display_channel_new(RedWorker *worker, int migrate, int stream_v
&cbs, dcc_handle_message);
spice_return_val_if_fail(display, NULL);
- stat_init(&display->add_stat, "add", red_worker_get_clockid(worker));
- stat_init(&display->exclude_stat, "exclude", red_worker_get_clockid(worker));
- stat_init(&display->__exclude_stat, "__exclude", red_worker_get_clockid(worker));
+ clockid_t stat_clock = red_worker_get_clockid(worker);
+ stat_init(&display->add_stat, "add", stat_clock);
+ stat_init(&display->exclude_stat, "exclude", stat_clock);
+ stat_init(&display->__exclude_stat, "__exclude", stat_clock);
#ifdef RED_STATISTICS
RedChannel *channel = RED_CHANNEL(display);
display->cache_hits_counter = stat_add_counter(channel->stat,
@@ -2063,13 +2050,13 @@ DisplayChannel* display_channel_new(RedWorker *worker, int migrate, int stream_v
display->non_cache_counter = stat_add_counter(channel->stat,
"non_cache", TRUE);
#endif
- stat_compress_init(&display->lz_stat, "lz", red_worker_get_clockid(worker));
- stat_compress_init(&display->glz_stat, "glz", red_worker_get_clockid(worker));
- stat_compress_init(&display->quic_stat, "quic", red_worker_get_clockid(worker));
- stat_compress_init(&display->jpeg_stat, "jpeg", red_worker_get_clockid(worker));
- stat_compress_init(&display->zlib_glz_stat, "zlib", red_worker_get_clockid(worker));
- stat_compress_init(&display->jpeg_alpha_stat, "jpeg_alpha", red_worker_get_clockid(worker));
- stat_compress_init(&display->lz4_stat, "lz4", red_worker_get_clockid(worker));
+ stat_compress_init(&display->lz_stat, "lz", stat_clock);
+ stat_compress_init(&display->glz_stat, "glz", stat_clock);
+ stat_compress_init(&display->quic_stat, "quic", stat_clock);
+ stat_compress_init(&display->jpeg_stat, "jpeg", stat_clock);
+ stat_compress_init(&display->zlib_glz_stat, "zlib", stat_clock);
+ stat_compress_init(&display->jpeg_alpha_stat, "jpeg_alpha", stat_clock);
+ stat_compress_init(&display->lz4_stat, "lz4", stat_clock);
display->n_surfaces = n_surfaces;
display->num_renderers = num_renderers;
diff --git a/server/display-channel.h b/server/display-channel.h
index 7cbc58d..e698788 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -201,10 +201,10 @@ struct DisplayChannel {
RedCompressBuf *free_compress_bufs;
/* TODO: some day unify this, make it more runtime.. */
-#ifdef RED_WORKER_STAT
stat_info_t add_stat;
stat_info_t exclude_stat;
stat_info_t __exclude_stat;
+#ifdef RED_WORKER_STAT
uint32_t add_count;
uint32_t add_with_shadow_count;
#endif
@@ -213,7 +213,6 @@ struct DisplayChannel {
uint64_t *add_to_cache_counter;
uint64_t *non_cache_counter;
#endif
-#ifdef COMPRESS_STAT
stat_info_t lz_stat;
stat_info_t glz_stat;
stat_info_t quic_stat;
@@ -221,7 +220,6 @@ struct DisplayChannel {
stat_info_t zlib_glz_stat;
stat_info_t jpeg_alpha_stat;
stat_info_t lz4_stat;
-#endif
};
#define LINK_TO_DCC(ptr) SPICE_CONTAINEROF(ptr, DisplayChannelClient, \
diff --git a/server/stat.h b/server/stat.h
index 5e8fc32..adbf46b 100644
--- a/server/stat.h
+++ b/server/stat.h
@@ -19,6 +19,7 @@
#define _H_STAT
#include <stdint.h>
+#include <glib.h>
typedef uint32_t StatNodeRef;
#define INVALID_STAT_REF (~(StatNodeRef)0)
@@ -53,12 +54,21 @@ static inline stat_time_t stat_now(clockid_t clock_id)
return ts.tv_nsec + (uint64_t) ts.tv_sec * (1000 * 1000 * 1000);
}
+typedef struct {
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
+ stat_time_t time;
+#endif
+} stat_start_time_t;
+
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
static inline double stat_cpu_time_to_sec(stat_time_t time)
{
return (double)time / (1000 * 1000 * 1000);
}
+#endif
-typedef struct stat_info_s {
+typedef struct {
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
const char *name;
clockid_t clock;
uint32_t count;
@@ -69,37 +79,54 @@ typedef struct stat_info_s {
uint64_t orig_size;
uint64_t comp_size;
#endif
+#endif
} stat_info_t;
-static inline void stat_reset(stat_info_t *info)
+static inline void stat_start_time_init(G_GNUC_UNUSED stat_start_time_t *tm,
+ G_GNUC_UNUSED const stat_info_t *info)
{
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
+ tm->time = stat_now(info->clock);
+#endif
+}
+
+static inline void stat_reset(G_GNUC_UNUSED stat_info_t *info)
+{
+#if defined(RED_WORKER_STAT) || defined(COMPRESS_STAT)
info->count = info->max = info->total = 0;
info->min = ~(stat_time_t)0;
#ifdef COMPRESS_STAT
info->orig_size = info->comp_size = 0;
#endif
+#endif
}
-#ifdef COMPRESS_STAT
-static inline void stat_compress_init(stat_info_t *info, const char *name, clockid_t clock)
+static inline void stat_compress_init(G_GNUC_UNUSED stat_info_t *info,
+ G_GNUC_UNUSED const char *name,
+ G_GNUC_UNUSED clockid_t clock)
{
+#ifdef COMPRESS_STAT
info->name = name;
info->clock = clock;
stat_reset(info);
+#endif
}
-static inline void stat_compress_add(stat_info_t *info,
- stat_time_t start, int orig_size,
- int comp_size)
+static inline void stat_compress_add(G_GNUC_UNUSED stat_info_t *info,
+ G_GNUC_UNUSED stat_start_time_t start,
+ G_GNUC_UNUSED int orig_size,
+ G_GNUC_UNUSED int comp_size)
{
+#ifdef COMPRESS_STAT
stat_time_t time;
++info->count;
- time = stat_now(info->clock) - start;
+ time = stat_now(info->clock) - start.time;
info->total += time;
info->max = MAX(info->max, time);
info->min = MIN(info->min, time);
info->orig_size += orig_size;
info->comp_size += comp_size;
+#endif
}
static inline double stat_byte_to_mega(uint64_t size)
@@ -107,32 +134,28 @@ static inline double stat_byte_to_mega(uint64_t size)
return (double)size / (1000 * 1000);
}
-#else
-#define stat_compress_init(a, b, c)
-#define stat_compress_add(a, b, c, d)
-#endif
-
-#ifdef RED_WORKER_STAT
-static inline void stat_init(stat_info_t *info, const char *name, clockid_t clock)
+static inline void stat_init(G_GNUC_UNUSED stat_info_t *info,
+ G_GNUC_UNUSED const char *name,
+ G_GNUC_UNUSED clockid_t clock)
{
+#ifdef RED_WORKER_STAT
info->name = name;
info->clock = clock;
stat_reset(info);
+#endif
}
-static inline void stat_add(stat_info_t *info, stat_time_t start)
+static inline void stat_add(G_GNUC_UNUSED stat_info_t *info,
+ G_GNUC_UNUSED stat_start_time_t start)
{
+#ifdef RED_WORKER_STAT
stat_time_t time;
++info->count;
- time = stat_now(info->clock) - start;
+ time = stat_now(info->clock) - start.time;
info->total += time;
info->max = MAX(info->max, time);
info->min = MIN(info->min, time);
+#endif
}
-#else
-#define stat_add(a, b)
-#define stat_init(a, b, c)
-#endif /* RED_WORKER_STAT */
-
#endif /* _H_STAT */
More information about the Spice-commits
mailing list