[Setup-tool-hackers] Speed problems in the tools.

[Miguel de Icaza <miguel ximian com>]

I was using the Ximian Setup Tools, and I noticed that "applying" a
change took way too much time.  I tracked down the problem, and it was
due to processing on the file descriptor handler the input from the
backend one character at a time.

Indeed, it processed one character on each invocation of the handler.
This was obviously extremely inneficient (there was a fixme there,
that said `it wont be too slow, because it is not too much data', but
even in this case it was plain inneficient).

Also, it seems like someone is trying to implement its own version of
the string handling routines, there is no reason to do so.  Please use
GString for this, not only it makes the code easier to read, it is an
API that has already been debugged.

I fixed the problem, and fixed some very broken code at the same time
(the magic "102k maximum of configuration" trick is gone).

Patch is attached.

Miguel.

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/ximian-setup-tools/src/common/ChangeLog,v
retrieving revision 1.88
diff -u -r1.88 ChangeLog
--- ChangeLog	2001/05/25 00:59:36	1.88
+++ ChangeLog	2001/05/29 23:24:29
@@ -1,3 +1,14 @@
+2001-05-29  Miguel de Icaza  <miguel@ximian.com>
+
+	* xst-tool.c (report_progress_tick): Process as much data as
+	possible in a single pass.
+
+	Removed ad-hoc string implementation from here. 
+	
+	(xst_tool_load): Use any pending data from the report pass as the
+	input to the XML document;  Make this work with arbitrarly large
+	XML documents.
+
 2001-05-24  Arturo Espinosa Aldama  <arturo@ximian.com>
 
 	* 0.5 RELEASE
Index: xst-tool.c
===================================================================
RCS file: /cvs/gnome/ximian-setup-tools/src/common/xst-tool.c,v
retrieving revision 1.34
diff -u -r1.34 xst-tool.c
--- xst-tool.c	2001/05/24 23:52:39	1.34
+++ xst-tool.c	2001/05/29 23:24:34
@@ -414,50 +414,67 @@
 {
 	XstTool *tool;
 	XstReportLine *rline;
-	char c;
+	char buffer [512];
+	int n, i = 0;
 
 	tool = XST_TOOL (data);
 
-#warning FIXME: make this not suck
-	if (!tool->line) {
-		tool->line = g_malloc(1024);
-		tool->line_len = 0;
-		tool->line [0] = '\0';
-	}
+	if (!tool->line)
+		tool->line = g_string_new ("");
 
 	/* NOTE: The read() being done here is inefficient, but we're not
 	 * going to handle any large amount of data */
 
-	if (read (fd, &c, 1) > 0) {
-		if (c == '\n') {
-			/* End of line */
+	while ((n = read (fd, buffer, sizeof (buffer)-1)) != 0){
 
-		        /* Report line; add to list */
-			rline = xst_report_line_new_from_string (tool->line);
+		buffer [n] = 0;
+		
+		for (i = 0; (i < n); i++){
+			char c = buffer [i];
 			
-			if (rline)
-				tool->report_line_list = g_slist_append (tool->report_line_list,
-									 rline);
-
-			tool->line [0] = '\0';
-			tool->line_len = 0;
-		} else {
-			/* Add character to end of current line */
-
-			tool->line = g_realloc (tool->line, MAX (tool->line_len + 2, 1024));
-			tool->line [tool->line_len] = c;
-			tool->line_len++;
-			tool->line [tool->line_len] = '\0';
+			if (c == '\n'){
+				/* End of line */
+				
+				/* Report line; add to list */
+				rline = xst_report_line_new_from_string (tool->line->str);
+				
+				if (rline)
+					tool->report_line_list = g_slist_append (
+						tool->report_line_list, rline);
+
+				g_string_assign (tool->line, "");
+
+				if (!tool->report_dispatch_pending){
+					report_dispatch (tool);
+					if (tool->report_finished){
+						i++;
+						goto full_break;
+					}
+				}
+			} else {
+				/* Add character to end of current line */
+				g_string_append_c (tool->line, buffer [i]);
+				
+			}
 		}
 	}
-	else
-	{
+
+ full_break:
+
+	if (n <= 0){
 		/* Zero-length read; pipe closed unexpectedly */
 		/* FIXME: handle this at the UI level correctly. */
 
 		tool->report_finished = TRUE;
 	}
 
+	if (tool->report_finished){
+		if (n > 0)
+			tool->xml_document = g_string_new (&buffer [i]);
+		g_string_free (tool->line, TRUE);
+		tool->line = NULL;
+	}
+	
 	if (!tool->report_dispatch_pending)
 		report_dispatch (tool);
 	
@@ -605,7 +622,6 @@
 {
 	int fd [2];
 	int t, len;
-	char *p;
 
 	g_return_val_if_fail (tool != NULL, FALSE);
 	g_return_val_if_fail (XST_IS_TOOL (tool), FALSE);	
@@ -629,6 +645,8 @@
 	if (t < 0) {
 		g_error ("Unable to fork.");
 	} else if (t) {
+		char buffer [2048];
+		
 		/* Parent */
 
 		close (fd [1]);	/* Close writing end */
@@ -643,22 +661,20 @@
 		if (tool->run_again)
 			return TRUE;
 
-		p = g_malloc (102400);
+		if (tool->xml_document == NULL)
+			tool->xml_document = g_string_new ("");
+		
 		fcntl(fd [0], F_SETFL, 0);  /* Let's block */
-		len = 0; 
-		for (len = 0; (t = read (fd [0], p + len, 102399 - len)); len += t)
-			;
-
-		if (len < 1 || len == 102399) {
-			g_free (p);
-		} else {
-			p = g_realloc (p, len + 1);
-			*(p + len) = 0;
-
-			tool->config = xmlParseDoc (p);
-			g_free (p);
-			close (fd [0]);
+
+		while ((t = read (fd [0], buffer, sizeof (buffer)-1)) != 0){
+			buffer [t] = 0;
+			g_string_append (tool->xml_document, buffer);
 		}
+		
+		tool->config = xmlParseDoc (tool->xml_document->str);
+		g_string_free (tool->xml_document, TRUE);
+		tool->xml_document = NULL;
+		close (fd [0]);
 	} else {
 		/* Child */
 
Index: xst-tool.h
===================================================================
RCS file: /cvs/gnome/ximian-setup-tools/src/common/xst-tool.h,v
retrieving revision 1.21
diff -u -r1.21 xst-tool.h
--- xst-tool.h	2001/05/14 18:25:51	1.21
+++ xst-tool.h	2001/05/29 23:24:35
@@ -73,8 +73,9 @@
 	gboolean timeout_done;
 	gboolean report_list_visible;
 
-	char *line;
-	int line_len;
+	GString *line;
+	GString *xml_document;
+	
 	guint input_id;
 	GSList *report_line_list;
 	gboolean report_dispatch_pending;

_______________________________________________
setup-tool-hackers maillist  -  setup-tool-hackers@ximian.com
http://lists.ximian.com/mailman/listinfo/setup-tool-hackers