Re: [Qemu-devel] [PATCH for-2.8 v1 00/60] Modular build of trace event files

[Daniel P. Berrange]

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.

NB, the simpletrace backend requires a globally unique 32-bit integer ID
to be assigned to each trace event, so even with the approach you suggest
below we still need to be able to assign a global ID for each event.

So while your suggest below avoids having to pass around the dstate
arrays, which is nice, we still have to assign event id offsets to
each trace-event file in some manner TBD.

> 
> What about using global variables? Instead of the dstate array, each event 
> could
> have this on the "public" header:
> 
>   /* define for static state */
>   #define TRACE_EVENTNAME_ENABLED 1
>   /* pointer to event descriptor */
>   extern TraceEvent *TRACE_EVENTNAME;
>   /* variable with dynamic state */
>   extern bool ___TRACE_EVENTNAME_DSTATE;
> 
>   void trace_eventname(...) {
>       if (trace_event_get_stateTRACE_EVENTNAME_ENABLED && 
> ___trace_eventname_dstate) {
>           /* ... */
>       }
>   }
> 
> The use of event IDs on generic code can be adapted like this:
> 
>   #define trace_event_get_state_dynamic_by_id(id) \
>       (unlikely(trace_events_enabled_count) && \
>        (___ ## id ## _DSTATE))
> 
> And then we can concatenate all "trace-events" files to generate the .c files:
> 
>   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[...];
> 
> So updating a single "trace-events" file does not force a recompile of the 
> whole
> QEMU, but we retain the performance of the flat dstate array (now a per-event
> pointer) and the simpler flat structure for iteration based on event names.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|