Francois,
Thanks for the patch, I
will test it today.
You are right that a
tabular table will only display few events. But what about searching?
Are there any concepts in
TMF regarding doing search operations very fast? Like searching for an event of
special type with a field with a specified value? Wouldn’t it be useful
to have a filtering/searching framework?
Best regards,
Franz
From:
linuxtools-dev-bounces@xxxxxxxxxxx [mailto:linuxtools-dev-bounces@xxxxxxxxxxx] On Behalf Of Francois Chouinard
Sent: Monday, November 09, 2009
5:42 AM
To: Linux
Tools developer discussions
Subject: Re: [linuxtools-dev]
TmfEventContent data storage
Hi Franz,
> The problem is that the content now parses the fields in the constructor
> which means that the map of fields is generated even for events that are
> only used to index the trace.
Agree. I put the parseContent() call back in the getField*() methods so it is
only called on demand and if needed. This should speed up the indexing.
> Also I think that the map of field id’s to field values that is
stored
> in each event is a performance issue. Why do you not keep the access
> function with the integral id? When used from a GUI side it would also
> be much faster getting the field values by an integer than by a String.
> Because a GUI would normally get a list of labels for the columns and
> then fill it with fields. It makes no sense from my point of view having
> an accessor function by a field label which would end up in a map-lookup.
The event content is not handled solely by GUI tables. Some (LTTng) components
need to access fields individually and storage in a Map structure, indexed by
field ID, is more versatile in that case.
Also, it seems to me that it is safer to have the method parseContent() return
a Map of the parsed fields instead of an array. This way the FW doesn't have to
worry about [1] variable list of fields (missing or optional), [2] positional
errors, [3] incompatible array sizes (number of fields different than number of
parameters). All this can very well be handled by the parseContent()
implementor but it definitely imposes an extra constraints on him/her.
Finally, I like the possibility of augmenting the set of attributes/flags of
the event as it navigates through the application. A map gives us more
flexibility for this.
It is expected that there will be relatively very few events displayed in
tabular format at any time (a few tens, not millions) so the extra map lookup
time should not even be perceivable form the user standpoint. However, I agree
that removing the index-based field access function wasn't a good idea and I
put it back.
> And finally - why is data stored that often in the TMF class structure?
> The content holds a list of field labels and a reference to the type.
> The type holds a list of field labels. The content holds a map of field
> label to values. And finally the field itself holds a copy of the field
> label. That means that each field label is stored on 4 different places
> within the same object. For the content I think it would be much faster
> to store a type reference. The list of field names can then be retrieved
> by asking the type. And the map should be replaced by a list of fields
> (like it was before) that is lazy-parsed. This way indexing will speed-up
> a lot.
I removed the list of labels from TmfEventContent and used the TmfEventType
reference. This adds an indirection but only if individual fields need to be
accessed.
>From a modelling perspective, I think that it is not a bad thing if a field
knows its ID. In the big scheme of things, the field might initially be
accessed through Event, then Content, but it could potentially be handled out
of context and it wouldn't be easy to find out its ID if we had to navigate
through the structure, figure out its position, etc.
I changed the type of the fields map to <String, Object>. If you don't
care about the field ID then you only need to store the field content. If you
need the field ID, it can be stored as a TmfEventField instead (holding ID and
content).
I posted the updated patch in https://bugs.eclipse.org/bugs/show_bug.cgi?id=287562
.
Regards,
/fc