Re: [Qemu-devel] [PATCH v2 5/6] trace: remove use of event ID enums from APIs

[Daniel P. Berrange]

On Thu, Sep 15, 2016 at 02:20:05PM +0200, Lluís Vilanova wrote:
> Daniel P Berrange writes:
> 
> > On Thu, Sep 15, 2016 at 01:26:06AM +0200, Lluís Vilanova wrote:
> >> Daniel P Berrange writes:
> >> 
> >> > Since there will shortly be multiple event groups allowed,
> >> > we can no longer use the TraceEventID and TraceEventVCPUID
> >> > enums in the trace control APIs. There will in fact be
> >> > multiple distinct enums, and the enum values will only be
> >> > required to be unique per group.
> >> 
> >> This patch serves no purpose without the event group patches.
> >> 
> >> Also, AFAIR TraceEventVCPUID still needs to be a flat space (they're all 
> >> used as
> >> bitmask indexes), so keeping the enum won't lose any re-compilation 
> >> benefit.
> >> 
> >> And without wanting to sound like a broken record, you can make the
> >> "TRACE_${EVENTNAME}" IDs be global Event* variables (statically 
> >> initialized in
> >> "trace/generated-events.c"). That still allows using their names in the 
> >> macros,
> >> avoids having a (two-level) tree of events, and eliminates the need for the
> >> Event::id member (and the trace_event_get_id() function).
> 
> > Regardless of whether its used in the public API, we still need to
> > be able to assign a unique 32bit event ID to every event, because
> > simpletrace currently needs to output that in its trace files.
> 
> > That said, i have been wondering if people are very tied to the
> > current simple trace output format ?  My ideal solution would be
> > for us to dynamically assign id values to trace events when QEMU
> > starts up.
> 
> > If we did this, hen in the simpletrace output file, we would have
> > to insert a new record type which records an (event name, ID)
> > value mapping, the first time any individual event type is
> > emitted.
> 
> > Thus when simpletrace.py comes to load the trace data, instead
> > of using the event ID to lookup the event directly, it would use
> > the event ID to get the name from the trace data, and then lookup
> > the event based on name.
> 
> A self-describing simpletrace would be so much better... Instead of 
> registering
> event types the first time they're emitted, it'd be more efficient (and 
> probably
> simpler) to register them during initialization of the trace subsystem.

We can easily make it self-describing enough that you don't
need to read 'trace-events' in order to parse the data. Whether
we go further and also include the arg names and format strings
to enable analysers to process the data, I'm not sure. We can
do it incrementally I guess.

THe only reason I suggested at time of first use is that we
have quite a large number of events and so emitting description
for all of them would take non-negligble time and possibly use
significant space. I will investigate further to see if this
is genuinely a problem.


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