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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Wed, Jun 14, 2017 at 12:45 PM Christophe de Dinechin <cdupontd@xxxxxxxxxx> wrote:

> On 13 Jun 2017, at 12:17, Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> wrote:
>

>> - Better memory locality (not wasting a whole cache line for a single flag)
>
> minor, because log is not for hot traces.

My experience differs from yours, and as a result, the idea that “log is not for hot traces" seems particularly wrong to me. In a real-time system, if your logging system cannot properly instrument the hot paths, then your logging system is at fault, not the idea of instrumenting the hot paths, and even less the idea of designing a mechanism capable of efficiently instrumenting hot paths.

Since Spice is a real-time system, it would have expected (and I really want, sooner better than later): 

a) a logging system that knows how to record stuff and print it out efficiently.
b) a tracing system that lets me view selected things as they happen, including in hot paths
c) a flight-recorder recording information continuously and dumping useful data after specific events, e.g. crash, signal, assert, etc.
d) performance probes, to continuously measure key things and report them
e) a tuning / tweaking system, to adjust internal parameters, ideally on a running system, and observe the effect

As far as I can tell, only (a) logging exists today. Why don’t we have the rest? And why is there such a strong push-back from you when I suggest we add it?
 
There is no push back, we discuss the best tool for the job and what changes are really required in the code. It's not obvious or easy. You can always send patches, talking and complaining is cheap.

>
> 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.

Actually, this argument is valid for any mechanism designed to also instrument hot paths. The first level cache is a very precious commodity. Say we have 50 categories in the normal run path. With your approche, we use 50 unrelated cache lines with nothing else of interest in them (the name and description of the category being largely uninteresting at run time). With mine, it’s one cache line, with the first category test priming the cache for subsequent category tests, ensuring they don’t stall.

Let’s design a tracing selection mechanism that can instrument hot paths at minimal cost, rather than rationalize why we don’t need to instrument hot paths.


There are better tools for this job! And let's not fight over performance numbers without any *real* use case benchmark (in all cases, printf or glog are not for hot traces)



>
>
>> - 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)

When splitting projects, which remains quite infrequent, both projects can inherit all categories as a starting point anyway.


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

On a daily basis, this one will matter. Getting errors faster, with more precise file / line locations, etc.>

If I read your code right, everything will be rebuilt when you add or change a category. It's worse imho.



>
>> - Does not require glib at all, so would work e.g. in Windows driver
>
> drivers have different means to log/trace.

Is that a good thing? Facilities can be designed to work the same way in kernel and user space, and when they are, it’s better.


No, you'll get push back for the same reasons, there are existing and better solutions for each domain.
 

>
>> - 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,

I dont’ know who “you” is here, but that’s not me. Example: showing available categories and their values in debugger is ‘p spice_traces’ in my approach.


Listing all categories is not harder than (gdb) info variables spice_log. You can then inspect or loop over those variables

Though I really don't get why you would want to see the whole state simultaneously...

--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]