Ethereal-dev: Re: [Ethereal-dev] compare_proto_id taking up 20% of Ethereal's time
On Sun, Nov 16, 2003 at 04:09:10PM -0800, Richard Sharpe wrote:
> It seems to me that we cannot simply change a proto_id into a pointer but
> cast it to an int, as that will cause problems on 64-bit platforms.
I wasn't suggesting that we do that.
> So, I want to change all those routines that currently deal with a
> proto_id to deal with a protocol_t * ...
No, just change the ones that use "find_protocol_by_id()" - or,
initially, just change the expensive ones
("proto_get_protocol_short_name()" and "proto_is_protocol_enabled()").
Don't change the "proto_add_xxx" ones. (The other ones that use
"find_protocol_by_id()" should probably be changed as well, for
consistency reasons rather than performance reasons.)
> But that requires changes to lots of places, including all dissectors
> (although the changes will be minor in each of them).
Most dissectors don't call the ones that call "find_protocol_by_id()".
They mainly use the protocol ID to add the top-level item to the
protocol tree; the "protocol ID" for a protocol is just an hf_ value,
and the routines used to add top-level items to the protocol tree just
take an hf_ value as an argument, so those routines should continue to
use protocol IDs. hf_ values are fairly quick to look up (they're
looked up in a big array) - they have to be, because those have to be
looked up for *every named field*.
Yes, this means that we use protocol IDs when adding items to the
protocol tree, and "protocol_t *"s in some other cases. So it goes....
See the changes I've just checked in. They changed the user-mode CPU
time for reading in one large capture I have from 139 seconds to 68
seconds.
(In the process of doing them, I found several dissectors that were
calling "proto_is_protocol_enabled()" when they didn't need to - a
dissector called through a handle, which includes dissectors called
through dissector tables, or a heuristic dissector isn't even called if
its protocol is disabled, so those dissectors don't need to check
whether their protocols are enabled. They also don't need to set
"pinfo->current_proto" - that's done for them also.
I also fixed some dissectors that were being called directly to be
called through handles.
There are still some dissectors that are calling
"proto_is_protocol_enabled()" and/or "proto_get_protocol_short_name()";
they currently explicitly call "find_protocol_by_id()". Many of them
should probably be fixed either not to call those routines at all, or to
save "protocol_t *"s for the protocols in question. They're probably
not for heavily-used protocols, so they don't need to be fixed
immediately.)
