"and from a vgprof output most of the time (ok it's the instruction count
but ethereal is so hard on the cache that it's not a too bad proxy) is
in building and destroying tree nodes."
In that case, maybe something worth testing would be to test this approach:
1, add a new field header_field_info.ref_count that keeps track of how
many times
a field is referenced from a filter of some sort. (similar to your patch)
0 means it is not referenced at all.
2, add code in proto_tree_add_item() and friends to test if ref_count
is zero or not.
If tree->visible==0 and ref_count is zero, then proto_tree_add_item()
and friends just return NULL without doing anything.
This would then make all expensive proto_tree_add_...() functions
return almost immediately unless
1, the tree is visible and to be displayed
2, tethereal is run with the -V option
3, the field is referenced by a filter and thus have to be evaluated
and put in the edt tree.
Then all dissectors would get a speedup without modifying them.
Would that be a good first step in optimizing performance?
As a second step one can later add code that keeps track of whether
any fields within that protocol is referenced or not by any filter and if not
then one can let the call_dissector_...() and friends check : are any
fields for this dissector
referenced by a filter or not, and if not, then just call the next
dissector with tree==NULL
thus again getting the same effect as your patch but without adding
the code explicitely to reset tree to NULL in the dissector itself.
This might require a new protocol registration routine to indicate
which protocols this is safe to do for (==protocols verified to not
call any functions from other protocols directly)
On Mon, 21 Mar 2005 06:24:51 +0000, didier <dgautheron@xxxxxxxx> wrote:
> ronnie sahlberg wrote:
> > Interesting.
> >
> > Four comments:
> >
> > 1, One would have to be a bit careful with where one uses
> > proto_tree_is_null() since there are instances where one protocol
> > dissector calls functions/subdissectors from other protocols. For
> > example NLM will call the dissector for NFS filehandles from the NFS
> > dissector. This would need to be documented. Can you write a README
> > for discussion on this?
> I'll look at it.
> >
> > 2, proto_tree_is_null() would need a better more appropriate name.
> :)
> >
> > 3, comment in your patch: i really think you need to do refcounting
> > for these items. I am not sure, but one might do refcounting just to
> > be on the safe side.
> > The refcounting might be as easy to implement as just
> > incrementing/decrementing hfinfo->infilter and letting ==0 mean not
> > used at all in any filter. Would need to check that that would work.
> infilter is a tristate -1 is used for filtering on protocol type. Could
> use some ugly bitfield though
> >
> > 4, proto_tree_is_null() is a too blunt instrument, it either enables
> > all fields or none fields. In addition to that one might also consider
> > putting
> > if(!( ->visible or ->infilter ))return
> > at the beginning of the proto_tree_add_ functions as well to further
> > reduce the number of calls made.
> > This might reduce the filter overhead even further.
> Yes but:
> 1) it will affect all dissectors, no idea if it's good or bad
> 2) not sure about tap stuff.
> 3) you can't do it at the beginning because then you return a NULL
> pointer and it breaks stuff like
> ti = proto_tree_add_item(tree, hf_val1 ...
> /* ti is NULL */
> s_tree = proto_item_add_subtree(ti, ett);
> /* s_tree is NULL */
> proto_tree_add_uint_hidden(s_tree, hf_val2...
> /* oops if you need hf_val2*/
>
> and from a vgprof output most of the time (ok it's the instruction count
> but ethereal is so hard on the cache that it's not a too bad proxy) is
> in building and destroying tree nodes.
>
> Didier
>
>
