[OpenAFS-devel] Ideas on vos dump slowness

Neulinger, Nathan R. nneul@umr.edu
Fri, 16 Mar 2001 14:21:43 -0600


This message is in MIME format. Since your mail reader does not understand
this format, some or all of this message may not be legible.

------_=_NextPart_000_01C0AE56.B7745990
Content-Type: text/plain;
	charset="ISO-8859-1"

Well, unfortunately, it didn't make any difference... RX must be doing some
buffering internally or something like that, or that isn't where the problem
is. Was worth a shot though. I've attached the diff I was playing with in
case anyone wants to try something similar. 

I did notice that if you made the buffer very large (i.e. like 256K) it
flips out volserver... must be something to do with the thread stack size
limits or something like that. 

-- Nathan

> -----Original Message-----
> From: Neulinger, Nathan R. [mailto:nneul@umr.edu]
> Sent: Friday, March 16, 2001 1:29 PM
> To: 'openafs-devel@openafs.org'
> Subject: [OpenAFS-devel] Ideas on vos dump slowness
> 
> 
> As near as I can tell, the key is in dumpstuff.c in volser. 
> Basically, the
> dump is calling iod_Write for each tiny little piece of data that is
> retrieved from the volume. Each iod_Write in turn immediately calls an
> rx_Write. I tried bumping up the block size for the read in 
> DumpFile, which
> might help for a volume containing really large files, 
> however, it doesn't
> help for volumes containing lots of tiny files.
> 
> It appears to me that this generates tons of rx calls (as the 
> code appears
> to name them) in the rx portion of the code, probably each requiring
> bidirectional communication of some sort. 
> 
> What I wonder is - what if iod_Write were to buffer? i.e. 
> have iod_Write
> keep a chunk of memory around to buffer up a 100k or so 
> before it calls
> rx_Write. Add a iod_Flush routine that would force iod_Write 
> to dump out
> it's buffer (this could be called at the end of the dump to insure
> everything has been sent.) Whenever it fills up the buffer, 
> go ahead and
> dump that out. 
> 
> I don't know enough about rx to know if this makes sense or 
> not, but given
> that use of iod_Write appears to be isolated to dumpstuff.c I 
> think I may
> give it a shot and see what happens. 
> 
> One negative is that write errors will be delayed till when the buffer
> actually gets flushed instead of returning immediately, but 
> given that the
> code doesn't distinguish between error locations currently, 
> that doesn't
> appear like a horrible problem. 
> 
> -- Nathan
> 
> ------------------------------------------------------------
> Nathan Neulinger                       EMail:  nneul@umr.edu
> University of Missouri - Rolla         Phone: (573) 341-4841
> Computing Services                       Fax: (573) 341-4216
> _______________________________________________
> OpenAFS-devel mailing list
> OpenAFS-devel@openafs.org
> https://lists.openafs.org/mailman/listinfo.cgi/openafs-devel
> 


------_=_NextPart_000_01C0AE56.B7745990
Content-Type: application/octet-stream;
	name="iod-buffer.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="iod-buffer.diff"

Index: vos.c=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
RCS file: /cvs/openafs/src/volser/vos.c,v=0A=
retrieving revision 1.2=0A=
diff -u -r1.2 vos.c=0A=
--- vos.c	2000/11/04 10:06:34	1.2=0A=
+++ vos.c	2001/03/16 20:18:37=0A=
@@ -406,6 +406,7 @@=0A=
     long blksize;=0A=
     int ufdIsOpen =3D 0;=0A=
 =0A=
+system("date");=0A=
     /* Open the output file */=0A=
     if (!filename || !*filename) {=0A=
 	usd_StandardOutput(&ufd);=0A=
@@ -432,6 +433,7 @@=0A=
 =0A=
   error_exit:=0A=
     /* Close the output file */=0A=
+#if 0=0A=
     if (ufdIsOpen) {=0A=
        code =3D USD_CLOSE(ufd);=0A=
        if (code) {=0A=
@@ -440,6 +442,9 @@=0A=
 	  if (!error) error =3D code;=0A=
        }=0A=
     }=0A=
+#endif=0A=
+=0A=
+system("date");=0A=
 =0A=
     return(error);=0A=
 }   =0A=
Index: dumpstuff.c=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
RCS file: /cvs/openafs/src/volser/dumpstuff.c,v=0A=
retrieving revision 1.3=0A=
diff -u -r1.3 dumpstuff.c=0A=
--- dumpstuff.c	2000/12/13 21:43:45	1.3=0A=
+++ dumpstuff.c	2001/03/16 20:18:38=0A=
@@ -42,6 +42,8 @@=0A=
 =0A=
 extern int DoLogging;=0A=
 =0A=
+#define IOD_BUFSIZE 1=0A=
+=0A=
 /* This iod stuff is a silly little package to emulate the old qi_in =
stuff, which=0A=
    emulated the stdio stuff.  There is a big assumption here, that =
the=0A=
    rx_Read will never be called directly, by a routine like readFile, =
when=0A=
@@ -60,6 +62,8 @@=0A=
     int *codes;                 /* one return code for each call */=0A=
     char haveOldChar;		/* state for pushing back a character */=0A=
     char oldChar;=0A=
+	int buffer_used;=0A=
+	char buffer[IOD_BUFSIZE];=0A=
 };=0A=
 =0A=
 =0A=
@@ -86,17 +90,18 @@=0A=
     iodp->haveOldChar =3D 0;=0A=
     iodp->ncalls =3D 1;=0A=
     iodp->calls =3D (struct rx_call **) 0;=0A=
+	iodp->buffer_used =3D 0;=0A=
 }=0A=
 =0A=
 static void iod_InitMulti(struct iod *iodp, struct rx_call **calls, =
int ncalls,=0A=
 		   int *codes)=0A=
 {=0A=
-=0A=
-  iodp->calls =3D calls;=0A=
-  iodp->haveOldChar =3D 0;=0A=
-  iodp->ncalls =3D ncalls;=0A=
-  iodp->codes =3D codes;=0A=
-  iodp->call =3D (struct rx_call *) 0;=0A=
+	iodp->calls =3D calls;=0A=
+	iodp->haveOldChar =3D 0;=0A=
+	iodp->ncalls =3D ncalls;=0A=
+	iodp->codes =3D codes;=0A=
+	iodp->call =3D (struct rx_call *) 0;=0A=
+	iodp->buffer_used =3D 0;=0A=
 }=0A=
 =0A=
 /* N.B. iod_Read doesn't check for oldchar (see previous comment) =
*/=0A=
@@ -108,6 +113,7 @@=0A=
  * I don't think we want half the replicas to go bad just because one =
=0A=
  * connection timed out, but if they all time out, then we should give =
up. =0A=
  */=0A=
+/* call with nbytes =3D -1 to flush the buffer */=0A=
 static int iod_Write(struct iod *iodp, char *buf, int nbytes)=0A=
 {=0A=
   int code, i;=0A=
@@ -116,26 +122,55 @@=0A=
   assert ((iodp->call && iodp->ncalls =3D=3D 1 && !iodp->calls) ||=0A=
 	  (!iodp->call && iodp->ncalls >=3D 1 && iodp->calls));=0A=
 =0A=
-  if (iodp->call) {=0A=
-    code =3D rx_Write(iodp->call, buf, nbytes); =0A=
-    return code;     =0A=
+  /* buffer for now, unless nbytes =3D=3D -1, in which case we flush =
the=0A=
+	buffer */=0A=
+  if ( nbytes !=3D -1 && (iodp->buffer_used + nbytes <=3D IOD_BUFSIZE) =
)=0A=
+  {=0A=
+	memcpy(iodp->buffer + iodp->buffer_used, buf, nbytes);=0A=
+	iodp->buffer_used +=3D nbytes;=0A=
+// Log("used buffer for %d send, now using %d\n", nbytes, =
iodp->buffer_used);=0A=
+	return nbytes; /* act like it worked */=0A=
   }=0A=
-=0A=
-  for (i=3D0; i < iodp->ncalls; i++) {=0A=
-    if (iodp->calls[i] && !iodp->codes[i]) {=0A=
-      code =3D rx_Write(iodp->calls[i], buf, nbytes);=0A=
-      if (code !=3D nbytes) { /* everything gets merged into a single =
error */=0A=
-      	iodp->codes[i] =3D VOLSERDUMPERROR;  /* but that's exactly what =
the */=0A=
-      }       	 		           /* standard dump does, anyways */=0A=
-      else {=0A=
-      	one_success =3D TRUE;=0A=
-      }=0A=
-    }=0A=
-  } /* for all calls */=0A=
+  else /* we're flushing and sending */=0A=
+  {=0A=
+	int used =3D iodp->buffer_used;=0A=
+	iodp->buffer_used =3D 0;=0A=
+// Log("real send for buffer %d, and %d\n", used, nbytes);=0A=
+=0A=
+	  if (iodp->call) {=0A=
+		code =3D rx_Write(iodp->call, iodp->buffer, used);=0A=
+		if ( code !=3D used ) { return 0; }=0A=
+=0A=
+	    code =3D rx_Write(iodp->call, buf, nbytes); =0A=
+	    return code;=0A=
+	  }=0A=
+=0A=
+	  for (i=3D0; i < iodp->ncalls; i++) {=0A=
+	    if (iodp->calls[i] && !iodp->codes[i]) {=0A=
+		  code =3D rx_Write(iodp->calls[i], iodp->buffer, used);=0A=
+		  if ( code !=3D used ) {=0A=
+			iodp->codes[i] =3D VOLSERDUMPERROR;=0A=
+		  }=0A=
+			else=0A=
+			{=0A=
+				one_success =3D TRUE;=0A=
+			}=0A=
+=0A=
+	      code =3D rx_Write(iodp->calls[i], buf, nbytes);=0A=
+	      if (code !=3D nbytes) { /* everything gets merged into a single =
error */=0A=
+	      	iodp->codes[i] =3D VOLSERDUMPERROR;  /* but that's exactly =
what the */=0A=
+	      }       	 		           /* standard dump does, anyways */=0A=
+	      else {=0A=
+	      	one_success =3D TRUE;=0A=
+	      }=0A=
+	    }=0A=
+	  } /* for all calls */=0A=
+	=0A=
+	if (one_success)=0A=
+	  return nbytes;=0A=
+	else return 0;=0A=
 =0A=
-if (one_success)=0A=
-  return nbytes;=0A=
-else return 0;=0A=
+	}=0A=
 }=0A=
 =0A=
 static void iod_ungetc(struct iod *iodp, int achar)=0A=
@@ -566,7 +601,9 @@=0A=
 =0A=
 static int DumpEnd(register struct iod *iodp)=0A=
 {=0A=
-    return(DumpInt32(iodp, D_DUMPEND, DUMPENDMAGIC));=0A=
+	int code =3D DumpInt32(iodp, D_DUMPEND, DUMPENDMAGIC);=0A=
+	iod_Write(iodp, NULL, 0); /* flush the buffer */=0A=
+	return code;=0A=
 }=0A=
 =0A=
 /* Guts of the dump code */=0A=

------_=_NextPart_000_01C0AE56.B7745990--