Wireshark · Ethereal-dev: [Ethereal-dev] incomplete performance patch giving ~7% for tethereal testcase

["Pia Sahlberg" <piabar@xxxxxxxxxxx>]

Hotmail is not my friend.

I am not allowed to attach files with my inferior browser any more  but this
can easily be solved by hotmails friendly suggestion to upgrade to IE6.

Well, Internet Exploder wont run on my laptop.
Thanks hotmail.

Anyone know of a good replacement to use instead when traveling?  Someone 
that can support the very advanced features like attaching files with other 
browser than IE6?


Anyway,
Guy,

I modified proto.h and proto.c
a sample capture with tethereal and 35000 packets got ~7-8% faster
mainly due to the smaller number of recursive calls.

It adds a new parent  (prnt) pinter to the node struct.
This pointer makes thr struct more closer to a binary tree and makes
it easier to do non-recursive traversal.
It has different syntax to the parent pointer.
For the parent pointer,   it always points to the parent to the list of 
children.
The new prnt pointer will point to the parent pointer of the children list 
only for the first_child and all others just point to the previous child in 
the list.
I.e. more like a normal binary tree.


Now for the big question,
Does any code really rely on the curernt behvoious of the parent pointer or 
can
we replace it with the prnt pointer?  (to make it more like a bin tree and 
to save space)

The savings in time (7-8%) was by just rewriting proto_tree_free() to 
manually traverse the tree (as the prnt pointer allows us to do) and free 
the nodes.

There are also two traversal routines in proto.c (one of which is no longer 
called)  which could be rewritten to be non-recursive as well if the new 
syntax for the prnt pointer is compatible with current users.


I dont consider the patch finished,   it should be updated to completely 
replace the parent pointer with the prnt pointer and all users that 
explicitely
touch the structure/parent pointer should be updated.

The performance gain in tethereal (7-8%) is worth it.
Could anyone look at the patches (sorry for attaching it in text format)
and massage them to be applyable to the tree for em?
Im internet disadvantaged for the time being.

thanks.


At the end, I have attached them in uu format which might not be as mangled 
as
the friendly hotmail does


*** ../../ethereal-orig/epan/proto.h	2003-12-07 09:41:32.000000000 +1100
--- proto.h	2003-12-07 13:24:30.000000000 +1100
***************
*** 131,136 ****
--- 131,138 ----

 /* Each proto_tree, proto_item is one of these. */
 typedef struct _proto_node {
+ 	struct _proto_node *prnt;
+
 	struct _proto_node *first_child;
 	struct _proto_node *last_child;
 	struct _proto_node *next;





*** ../../ethereal-orig/epan/proto.c	2003-12-07 09:41:32.000000000 +1100
--- proto.c	2003-12-07 15:24:04.000000000 +1100
***************
*** 49,56 ****

 #define cVALS(x) (const value_string*)(x)

! static gboolean
! proto_tree_free_node(proto_node *node, gpointer data);

 static void fill_label_boolean(field_info *fi, gchar *label_str);
 static void fill_label_uint(field_info *fi, gchar *label_str);
--- 49,55 ----

 #define cVALS(x) (const value_string*)(x)

! static void free_node_tree_data(tree_data_t *tree_data);

 static void fill_label_boolean(field_info *fi, gchar *label_str);
 static void fill_label_uint(field_info *fi, gchar *label_str);
***************
*** 328,333 ****
--- 327,333 ----
 	return FALSE;
 }

+
 static gboolean
 proto_tree_traverse_in_order(proto_tree *tree, proto_tree_traverse_func 
func,
     gpointer data)
***************
*** 373,378 ****
--- 373,379 ----
 	return FALSE;
 }

+
 void
 proto_tree_children_foreach(proto_tree *tree, proto_tree_foreach_func 
func,
     gpointer data)
***************
*** 388,398 ****
 	}
 }

 /* frees the resources that the dissection a proto_tree uses */
 void
 proto_tree_free(proto_tree *tree)
 {
! 	proto_tree_traverse_in_order(tree, proto_tree_free_node, NULL);
 }

 static void
--- 389,449 ----
 	}
 }

+ #define FREE_NODE_FIELD_INFO(finfo)	\
+ 	if(finfo->rep){			\
+ 		ITEM_LABEL_FREE(finfo->rep);	\
+ 	}				\
+ 	FVALUE_CLEANUP(&finfo->value);	\
+ 	FIELD_INFO_FREE(finfo);
+
+
 /* frees the resources that the dissection a proto_tree uses */
 void
 proto_tree_free(proto_tree *tree)
 {
! 	proto_node *pnode = tree;
! 	proto_node *current, *parent;
! 	field_info *finfo;
!
! 	current = pnode->first_child;
! 	current->prnt=NULL;
! 	do {
! 		/* walk down all children first */
! 		if(current->first_child){
! 			current=current->first_child;
! 			continue;
! 		}
! 		/* then walk down the siblings */
! 		if(current->next){
! 			current=current->next;
! 			continue;
! 		}
!
! 		/* oh we are at a leaf, deallocate this one and update parents pointers 
*/
! 		parent=current->prnt;
! 		if(parent){
! 			if(parent->next==current){
! 				parent->next=NULL;
! 			} else if(parent->first_child==current){
! 				parent->first_child=NULL;
! 			}
! 		}
!
! 		finfo = PITEM_FINFO(current);
! 		if (finfo == NULL) {
! 			/* This is the root node. Destroy the per-tree data.
! 			 * There is no field_info to destroy. */
! 			free_node_tree_data(PTREE_DATA(current));
! 		} else {
! 			/* This is a child node. Don't free the per-tree data, but
! 			 * do free the field_info data. */
! 			FREE_NODE_FIELD_INFO(finfo);
! 		}
!
! 		/* Free the proto_node. */
! 		PROTO_NODE_FREE(current);
! 		current=parent;
! 	} while (current);
 }

 static void
***************
*** 417,451 ****
         g_free(tree_data);
 }

- #define FREE_NODE_FIELD_INFO(finfo)	\
- 	if(finfo->rep){			\
- 		ITEM_LABEL_FREE(finfo->rep);	\
- 	}				\
- 	FVALUE_CLEANUP(&finfo->value);	\
- 	FIELD_INFO_FREE(finfo);
-
- static gboolean
- proto_tree_free_node(proto_node *node, gpointer data _U_)
- {
- 	field_info *finfo = PITEM_FINFO(node);
-
- 	if (finfo == NULL) {
- 		/* This is the root node. Destroy the per-tree data.
- 		 * There is no field_info to destroy. */
- 		free_node_tree_data(PTREE_DATA(node));
- 	}
- 	else {
- 		/* This is a child node. Don't free the per-tree data, but
- 		 * do free the field_info data. */
- 		FREE_NODE_FIELD_INFO(finfo);
- 	}
-
- 	/* Free the proto_node. */
- 	PROTO_NODE_FREE(node);
-
- 	return FALSE; /* FALSE = do not end traversal of protocol tree */
- }
-
 /* Is the parsing being done for a visible proto_tree or an invisible one?
  * By setting this correctly, the proto_tree creation is sped up by not
  * having to call vsnprintf and copy strings around.
--- 468,473 ----
***************
*** 1856,1866 ****
 	pnode->tree_data = PTREE_DATA(tree);

 	if (tnode->last_child != NULL) {
 		sibling = tnode->last_child;
 		g_assert(sibling->next == NULL);
 		sibling->next = pnode;
! 	} else
 		tnode->first_child = pnode;
 	tnode->last_child = pnode;

 	return (proto_item*)pnode;
--- 1878,1891 ----
 	pnode->tree_data = PTREE_DATA(tree);

 	if (tnode->last_child != NULL) {
+ 		pnode->prnt = tnode->last_child;
 		sibling = tnode->last_child;
 		g_assert(sibling->next == NULL);
 		sibling->next = pnode;
! 	} else {
! 		pnode->prnt = tnode;
 		tnode->first_child = pnode;
+ 	}
 	tnode->last_child = pnode;

 	return (proto_item*)pnode;
***************
*** 2111,2116 ****
--- 2136,2142 ----

 	/* Initialize the proto_node */
 	PROTO_NODE_NEW(pnode);
+ 	pnode->prnt = NULL;
 	pnode->parent = NULL;
 	pnode->finfo = NULL;
 	pnode->tree_data = g_new(tree_data_t, 1);


UUencoded tgz file for both patches

begin 644 proto.tgz
M'XL(`)NNTC\``^U8ZV_;-A#/5^VON&+`9CN2(_F=&.F0+@X0($N"+MF^#!`4
MB8J%::(AT4FS(/_[[DA*IFWET79M]O#!D"7RQ^/Q>"]REG/!VV$[2N)XZPN1
MZ[GNH-?;<A6M_'M>Q^MON</AH-?O]/O]+N([+C:!^Z4$,FE>B"`'V,HY%T_A
MGNO_EU*KU8)V>P=_3$Q9SH+4X7ERO<-F0;8S4\9A=5RWZW@=QQV"N[O7\_:Z
MG;9;$FQ[N+_?.(X#-7BOO]?I[;F]-7QKF>@;>KMV?P#R$P!_WT8L3C(&X2\'
M)S\W/C2A$?*L$'`3I'/F%R)/LNM6$SL(_@9P)T42PO45YRD+,FR1`ODB9\R/
MZ9'QB#54([U"BYXV7,]XD@F60Q2(H#E6DVMN-SR)($[2U$^#*Y;ZFGDC3E@:
M^4D6<VC%"?((IVA'+05"T22;1WC,<;:7,""=DD[Z@&_.Y^A$25"J0&F$%MNH
MWGP!K>KC'Z2#.COI=D9VM]M5ED)*ZG:&LD&KR<J9F.<9'*&*)B3#`RUGVUA1
M92-@VHC(@QN6%PPE\GD>L;RQZ%3:L6OA\3P+@1XV\B-:-JCZ-0R[=G<X,M8@
M&W9?L`92Y;+@X31)HYQE?LS1A</ITW)KT*>)/4+5[XY*)[4>2LD`=EK2P@K`
M0`(Y*_@\#^57(&13E!0%"T7",P@,<6!>(JU4[LP8KBV&#+M>[1MZ\FM6U]V
M:?XVG%Z>G#3'"]$-&U5[,=JU>[W%7CPL]%_ZW]'[R<0_/3N<^$?'DY-#__CT
MZ`SM&4VY:?V&."N)U:?S-F>SYKUEJ6;K^&+RDW]R\&YRXA,/$S16D`>K!!^A
MDU]._!]/)@>GE^>-[S16^GJ)7DQO\,/%;6MK>:UM40%V)O_V@0#CU<YPGJ/1
M"AMA`;U(P'),P#]JI0Z-1F:2J?,V3O)"*-L?&P#G[2S/Q#[ML6R.N!++0DW<
M!NGO$/%;7&N:0NDV(#G18@F&&U=Q,J9H*B;E+/MUF+&&\$PDV5PM&(U'3X[:
MS@P)2/E%<I5BP"[JYL[8!_'HI-3YQ&SEE'P*MPQ0N8"['0"&O-B&"+-\RL-`
M,)0A*8"C.0=9!/-91&UJ+PK0P:"23;7O+VEY7$JM.DMQJP8EZ'XYJ.RWEGJK
MG4+I@:4%`V.\H=W'V9@@DUNE$?DBK0G-YURZX)'TV))CN1!H:-2^"A+:=$B7
M%Z2K1/L15H-`5MB&0X:)BM_)YAG+'>D0%$#;:B302*RL:&C&P3!OP7$KY.!V
MJ6.K+D>?7U"L.3RX.*C$U?)J=:W+&"C;+D7DV?="1H%U*6VXFHM*4O25"F9(
M*I=3R?A$Z!LO:QPE.JIFK1R_XG3^_NSB3'.BT+6\&:7-&['A`6YQ60P,9&T,
MKRTNO:'=ZWMEYBKI6@6SY=)'LG1>&.N=^ECO/!_KG2K6.R^(]<[CL=XA:5=+
M&^>3RE_P+_TFCKVG^=9B\8KWT/AR^GKG<>`3?8<&OMAU"/R,YTA1I:QHGOC0
MCK,BW\?ZC9;R.;<AV)->HZ6B_R=\!GM7769I!Y9*1DK\\@WW#.7+4.D,@[PN
ME((4>*SXASP%E<5I!BF&K!J.U7:A^Q68HN"*T3.B9($%)&KJ)J'LQ<R:@=HS
M2+*R"\$_D*^UX-T=%$P(8B%S3LC1@4.1WMG&0B6/$(M368T@JI@Q2DMP=4?R
M*T[3X$9RX1!2#K\ILAF>>40L4UC(9SB1/`3A9N9\GD5M=8P:C.S>4)\0ZL*#
M-^H/;&\TJ,Z?EJXR*FLBVU_8DZQX]#%)FKY0\#0H,Q&\6;@!8BR=ZZD86H6.
M)>#:#[`4RT5#(U5ZK+QI;'(I^U0MI(,CV;0$B;4":8'$_G51C5[C[*%C12+8
M'ZVF!I`RO=%PA+K:]:H"^>_4%17)FA^5&(_KZ^LH5&?7&HG&S^IZ6QT=/DOA
M=<;:\3S/QL=@<7SL>-T!-O4ZQET!Q9+C+!%)D"9_KD845=N;`>5T\FMCI@/*
M-JRL6)55BZU66;FFH\P3J^VF;5S[&;LU[QYL\'#2U[X,^Q^2NK*;ON+]+_:Y
M@^K^MS=TY?UOI[.Y__T:](+[W^E'WO\NX;TNW?]VW1?=_WI=S\8PM@AJJF%D
MA#2,:),@G!H%0WG'0T$3]&D62QM<3:&J)@!QAW4$BZDPF(<"?",(RG13T]Y2
MIUMY@U+;OW3J?P2SDH_J(.H<_]I6L*$-;6A#&]K0AC:TH0UM:$,;^J_37XTY
&_=L`*```
`
end

_________________________________________________________________
Hot chart ringtones and polyphonics. Go to  
http://ninemsn.com.au/mobilemania/default.asp