[Spice-devel] [PATCH spice-common 0/4] RFC: add structured logging and log category

[Marc-André Lureau]

Hi

----- Original Message -----
> Global declarations pros:
> - Obeys the DRY principle, as opposed to WET category declarations
>   (e.g. SPICE_LOG_CATEGORY_DECLARE vs. SPICE_LOG_CATEGORY)

True, but SPICE_LOG_CATEGORY_DECLARE should not be needed, I'd rather remove it.

> - Makes it possible to organize categories logically, e.g. for help output

Alphabetical order should be enough if categories are correctly named. I can change that easily.

> - Easier to find what categories already exists (no need to grep all source)

minor, and you can use SPICE_DEBUG=help.

> - Category changes impact a single file, easier for history or merge

Or not, if for example splitting a project in subprojects (like libcacard in qemu etc)

> - Categories shared between client and server, easier e.g. for remote
> activation

Good category naming solves the sharing aspect.

> - Category errors (duplicate, missing) detected early by compiler, rather
> than linker

minor

> - Does not require glib at all, so would work e.g. in Windows driver

drivers have different means to log/trace.

> - Whole state represented as a single C struct, easy to save/restore, print
> in debugger, etc

Well, you are usually interested by a specific value, so it's also a bad idea to put everything together.

> - Better memory locality (not wasting a whole cache line for a single flag)

minor, because log is not for hot traces.

This argument is probably only valid if you have a mix of categories in a very tiny context. If you switch to a single category, there shouldn't be much performance difference locally and globally.

> > 
> > Talking about common things it seems we quite agree to not using
> > multiple domains calling GLib but just use "Spice". Maybe we can
> > code this as a preparation change before these changes.
> > 
> >> 
> >> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> >> 
> >> Hi,
> >> 
> >> This is a RFC series to clean-up Spice logging infrastructure to fully
> >> rely on glib g_log & g_log_structured if available, and add logging
> >> categories. It is thus a counter-proposal to Christophe D. "RFC:
> >> Lightweight tracing mechanism", with which it shares the category
> >> selection/listing feature.
> >> 
> >> It is useful to be able to filter the logging by domains, but GLib
> >> only provides domain selection with G_MESSAGES_DEBUG, and it's
> >> recommended to have few domains per application/library.  Also, domain
> >> filtering code in glib isn't very efficient, and can't be listed
> >> easily.
> >> 
> >> A common solution to this problem is to add some form of "sub-domain"
> >> or "categories" on top of glib (see for ex gtkdebug.h). I propose a
> >> simple way to register such log categories, to list and selectively
> >> enable them with the SPICE_DEBUG= environment variable, as well as
> >> related convenience macros.
> >> 
> >> Comments welcome,
> >> 
> >> Marc-André Lureau (4):
> >>  log: replace spice log with glib
> >>  Replace spice_* logging with g_*
> >>  RFC: Add spice log categories
> >>  Add 'common_ssl' Spice log
> >> 
> >> common/canvas_base.c        | 148 ++++++++++-----------
> >> common/canvas_utils.c       |  18 +--
> >> common/gdi_canvas.c         |  82 ++++++------
> >> common/log.c                | 208 +++++++----------------------
> >> common/log.h                | 175 ++++++++++++++----------
> >> common/lz.c                 |  22 +--
> >> common/lz_decompress_tmpl.c |  18 +--
> >> common/marshaller.c         |   2 +-
> >> common/mem.c                |   2 +-
> >> common/pixman_utils.c       | 156 +++++++++++-----------
> >> common/quic.c               |  88 ++++++------
> >> common/quic_family_tmpl.c   |   6 +-
> >> common/quic_rgb_tmpl.c      |  32 ++---
> >> common/quic_tmpl.c          |  32 ++---
> >> common/rect.h               |   2 +-
> >> common/region.c             |   2 +-
> >> common/ring.h               |  26 ++--
> >> common/rop3.c               |  10 +-
> >> common/snd_codec.c          |  18 +--
> >> common/ssl_verify.c         |  74 ++++++-----
> >> common/sw_canvas.c          |  20 +--
> >> tests/test-logging.c        | 317
> >> +-------------------------------------------
> >> 22 files changed, 544 insertions(+), 914 deletions(-)
> >> 
> > 
> > Frediano
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > <mailto:Spice-devel at lists.freedesktop.org>
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > <https://lists.freedesktop.org/mailman/listinfo/spice-devel>
>