Re: [Qemu-devel] [PATCH] trace: Provide a per-event status define for conditional compilation

[Lluís Vilanova]

Blue Swirl writes:

> 2011/9/21 Lluís Vilanova <address@hidden>:
[...]
>> +== Trace event properties ==
>> +
>> +Each event in the "trace-events" file can be prefixed with a space-separated
>> +list of zero or more of the following event properties.
>> +
>> +=== "disable" ===
>> +
>> +If a specific trace event is going to be invoked a huge number of times, 
>> this
>> +might have a noticeable performance impact even when the event is
>> +programmatically disabled.
>> +
>> +In this case you should declare such event with the "disable" property. This
>> +will effectively disable the event at compile time (by using the "nop" 
>> backend),
>> +thus having no performance impact at all on regular builds (i.e., unless you
>> +edit the "trace-events" file).

> Nack, by default events should have no performance impact.

Sure, but some events can have an impact. Then it is advisable to ship QEMU with
the "disable" property on these events, and (if necesary) use the
trace_*_enabled (or TRACE_*_ENABLED) define to eliminate all possible sources of
performance degradation from that event.

For example, the patches I sent long ago for instrumenting guest code using TCG
now use QEMU's tracing infrastructure. These events are disabled by default, but
some of them perform some computations to produce their arguments. E.g., one of
the events has code for computing the set of used/defined registers for each
instruction, which is not for free.


[...]
>> diff --git a/scripts/tracetool b/scripts/tracetool
>> index 4c9951d..97f9f1b 100755
>> --- a/scripts/tracetool
>> +++ b/scripts/tracetool
>> @@ -519,7 +519,7 @@ linetostap_end_dtrace()
>>  # Process stdin by calling begin, line, and end functions for the backend
>>  convert()
>>  {
>> -    local begin process_line end str disable
>> +    local begin process_line end str name enabled
>>     begin="lineto$1_begin_$backend"
>>     process_line="lineto$1_$backend"
>>     end="lineto$1_end_$backend"
>> @@ -534,8 +534,14 @@ convert()
>>         # Process the line.  The nop backend handles disabled lines.
>>         if has_property "$str" "disable"; then
>>             "lineto$1_nop" "$str"
>> +            enabled=0
>>         else
>>             "$process_line" "$str"
>> +            enabled=1

> Instead of '1', couldn't this be a function that checks if the trace
> point is enabled during run time (from monitor etc.)?

What you're proposing is the counterpart of 'trace_event_set_state' (in
"trace/control.h"). This would miss the whole point of this patch, which is to
be able to ship QEMU with relatively costly tracing events whose performance
impact can be completely eliminated at compile time.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth