Daniel P Berrange writes:
> On Thu, Sep 08, 2016 at 03:23:26PM +0200, Lluís Vilanova wrote:
>> Daniel P Berrange writes:
>>
>> > I previously split the global trace-events file up into one file
>> > per-subdirectory to avoid merge conflict hell.
>> [...]
>>
>> Sorry, I could not find the message where the infrastructure is modified to
>> provide this. But I think there's a more efficient way to provide modular
>> auto-generated tracing code without the hierarchical indexing you proposed.
> [snip]
>> struct TraceEvent ___trace_events[] = {
>> {
>> .name = "eventname",
>> .sstate = 1,
>> .dstate = ___trace_eventname_dstate;
>> }
>> }
>>
>> TraceEvent *TRACE_EVENTNAME = &___trace_events[...];
> Life would be simpler if we had the 'bool dstate' as part of the
> TraceEvent struct, but doing so would essentially be reverting this
> previous change:
> commit 585ec7273e6fdab902b2128bc6c2a8136aafef04
> Author: Paolo Bonzini <pbonz...@redhat.com>
> Date: Wed Oct 28 07:06:27 2015 +0100
> trace: track enabled events in a separate array
> This is more cache friendly on the fast path, where we already have
> the event id available.
> I asked Paolo about this previously and he indicated it was a notable
> performance improvement, so we can't put dstate back into the TraceEvent
> struct :-(
Sorry, there was a typo in my example code that led to this misunderstanding.
I meant this for the global auto-generated .c:
struct TraceEvent {
/* ... */
bool *dstate;
};
bool ___TRACE_EVENTNAME_DSTATE;
struct TraceEvent ___trace_events[] = {
{
.name = "eventname",
.sstate = 1,
.dstate = &___TRACE_EVENTNAME_DSTATE;
}
}
TraceEvent *TRACE_EVENTNAME = &___trace_events[...];
If you look at the modified macro I pasted for "trace/control-internal.h":
#define trace_event_get_state_dynamic_by_id(id) \
(unlikely(trace_events_enabled_count) && \
(___ ## id ## _DSTATE))
We're retaining the fast-path performance of the seggregated boolean dstate
array, while the event descriptor contains a pointer to the per-event dstate
global boolean variable in case you want to get/set the dstate based on an event
pointer.
Cheers,
Lluis
