Here's another "startup speed" patch (against plib this time) for
windows users to try.

The plib loader for the RGB texture format wants to do piecewise I/O
on the file.  It will repeatedly seek to the beginning of a scan line
in the image, then read, then seek, etc...  Because of the color
separation in the image format, this adds up to *thousands* of I/O
syscalls per texture.

Even worse, (under linux at least, although it's likely that all
platforms have similar behavior) the C library's fread()
implementation (1) buffers I/O by trying to read a 4k page at a time,
and (2) flushes this buffer when you do a seek.  So the reader ends up
reading many parts of the file several times (about a 3x overhead on
the file I looked at).

So attached is a patch against the plib loader that reads the whole
file as a single chunk.  In combination with the airport data fix,
this has reduced the number of system calls during startup by about
90%.  Hopefully this will help the cygwin users out there.

Under linux, again, this doesn't change much.  I see at best about a
6% increase over the code in CVS.  Linux syscalls are very fast. :)

A final note: who else knows that there are *two* parsers for the RGB
image file format linked into the FlightGear binary?  One is this one
from plib; the other is in SimGear, and is used (as far as I can see)
only for the splash screen texture.  They appear to be descended from
the same code; is there any reason we shouldn't remove the one in
SimGear?

Anyway, let me know if this produces any appreciable speedup under
windows, and we can start the, ahem, bureacratic process of getting
this into plib. :)

Andy
Index: src/ssg/ssgLoadSGI.cxx
===================================================================
RCS file: /cvsroot/plib/plib/src/ssg/ssgLoadSGI.cxx,v
retrieving revision 1.17
diff -u -r1.17 ssgLoadSGI.cxx
--- src/ssg/ssgLoadSGI.cxx	14 Mar 2004 11:26:56 -0000	1.17
+++ src/ssg/ssgLoadSGI.cxx	27 May 2005 00:42:21 -0000
@@ -60,7 +60,19 @@
   int           isSwapped;
   unsigned char *rle_temp;
   bool          loadSGI_bool;
+  char          *image_buf;
+  int           seek_pos;
 
+  // This code was originally written to do piecewise I/O on the input
+  // file, but the number of syscalls involved caused performance
+  // problems in FlightGear on windows.  These functions mimic the
+  // original fseek/fread interface but do the actual work from a
+  // single buffer.
+  void doSeek(int pos) { seek_pos = pos; }
+  void doRead(void* buf, int sz) {
+    memcpy(buf, image_buf+seek_pos, sz);
+    seek_pos += sz;
+  }
 
   ssgSGIHeader () ;
   ssgSGIHeader(const char *fname, ssgTextureInfo* info ); 
@@ -159,14 +171,14 @@
 unsigned char ssgSGIHeader::readByte ()
 {
   unsigned char x ;
-  fread ( & x, sizeof(unsigned char), 1, image_fd ) ;
+  doRead ( & x, sizeof(unsigned char) );
   return x ;
 }
 
 unsigned short ssgSGIHeader::readShort ()
 {
   unsigned short x ;
-  fread ( & x, sizeof(unsigned short), 1, image_fd ) ;
+  doRead ( & x, sizeof(unsigned short) );
   swab_short ( & x ) ;
   return x ;
 }
@@ -174,7 +186,7 @@
 unsigned int ssgSGIHeader::readInt ()
 {
   unsigned int x ;
-  fread ( & x, sizeof(unsigned int), 1, image_fd ) ;
+  doRead ( & x, sizeof(unsigned int) );
   swab_int ( & x ) ;
   return x ;
 }
@@ -185,7 +197,7 @@
   if ( y >= ysize ) y = ysize - 1 ;
   if ( z >= zsize ) z = zsize - 1 ;
 
-  fseek ( image_fd, start [ z * ysize + y ], SEEK_SET ) ;
+  doSeek ( start [ z * ysize + y ] ) ;
 
   if ( type == SGI_IMG_RLE )
   {
@@ -193,7 +205,7 @@
     unsigned char *bufp = buf ;
     int length = leng [ z * ysize + y ];
 
-    fread ( rle_temp, 1, length, image_fd ) ;
+    doRead ( rle_temp, length );
 
     unsigned char pixel, count ;
 
@@ -221,7 +233,8 @@
     }
   }
   else
-    fread ( buf, 1, xsize, image_fd ) ;
+    //fread ( buf, 1, xsize, image_fd ) ;
+    doRead ( buf, xsize ) ;
 }
 
 
@@ -256,6 +269,8 @@
   leng  = NULL ;
   rle_temp = NULL ;
   image_fd = NULL ;
+  image_buf = 0;
+  seek_pos = 0;
 }
 
 ssgSGIHeader::ssgSGIHeader ( const char *fname, ssgTextureInfo* info )
@@ -264,6 +279,8 @@
 
   start = NULL ;
   leng = NULL ;
+  image_buf = 0;
+  seek_pos = 0;
 
   bool success=openFile(fname);
 
@@ -374,6 +391,8 @@
   
   if (image_fd != NULL)
     fclose(image_fd);
+
+  delete[] image_buf;
 }
 
 
@@ -451,12 +470,19 @@
     return false ;
   }
 
+  fseek(image_fd, 0, SEEK_END);
+  unsigned int sz = ftell(image_fd);
+  fseek(image_fd, 0, SEEK_SET);
+  image_buf = new char[sz];
+  if(fread(image_buf, 1, sz, image_fd) != sz)
+    return false;
+
   sgihdr -> readHeader () ;
 
   if ( sgihdr -> type == SGI_IMG_RLE )
   {
-    fread ( sgihdr->start, sizeof (unsigned int), sgihdr->tablen, image_fd ) ;
-    fread ( sgihdr->leng , sizeof (int), sgihdr->tablen, image_fd ) ;
+    doRead ( sgihdr->start, sizeof (unsigned int) * sgihdr->tablen ) ;
+    doRead ( sgihdr->leng , sizeof (int) * sgihdr->tablen ) ;
     swab_int_array ( (int *) sgihdr->start, sgihdr->tablen ) ;
     swab_int_array ( (int *) sgihdr->leng , sgihdr->tablen ) ;
 
_______________________________________________
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d

Reply via email to