Opened 15 years ago

Last modified 15 years ago

#281 closed defect (fixed)

RasterIO speedup patch

Reported by: tbeckman@… Owned by: dron
Priority: high Milestone:
Component: default Version: unspecified
Severity: normal Keywords:
Cc:

Description

The following patch for gdal/core/rasterio.cpp speeds up RasterIO significantly 
when it needs to do data type conversions.  For a 100MB test case of converting 
a Float32 image to Byte, the processing time goes from 49 seconds to 27 seconds 
on our machine.

--- ../../../remotesensing_gdal/core/rasterio.cpp	Mon Nov 11 10:02:06 2002
+++ rasterio.cpp	Wed Feb  5 15:58:02 2003
@@ -103,6 +113,8 @@
 
 {
     int         nBandDataSize = GDALGetDataTypeSize( eDataType ) / 8;
+    int bytesPerDataPixel = GDALGetDataTypeSize( eDataType ) / 8;
+    int bytesPerBufPixel = GDALGetDataTypeSize( eBufType ) / 8;
     GByte       *pabySrcBlock = NULL;
     GDALRasterBlock *poBlock;
     int         nLBlockX=-1, nLBlockY=-1, iBufYOff, iBufXOff, iSrcY;
@@ -231,6 +243,26 @@
                     return CE_Failure;
             }
 
+            /* calculate buffer span in the current block and handle as many
+               pixels as possible in one call to the GDALCopyWords routine.
+               Note: this makes sure that if the data is being subsampled at
+               somethings besides an integer interval that only one pixel at a
+               time is grabbed.  This is very inefficient.  It would likely be
+               better to convert the entire line, then pull out the pixels that
+               are needed for the subsampled version. */
+            int iSrcXInc = (int)dfSrcXInc;
+            int wordCount = 1;
+            if ( dfSrcXInc == iSrcXInc)
+            {
+                /* integer multiple for subsampling, so calculate the number
+                   of words to copy.  Also make sure the end of the requested
+                   bytes isn't exceeded. */
+                int endXBlockIndex = (nLBlockX + 1) * nBlockXSize;
+                wordCount = (endXBlockIndex - iSrcX)/iSrcXInc;
+                if (wordCount > (nBufXSize - iBufXOff))
+                    wordCount = nBufXSize - iBufXOff;
+            }
+
 /* -------------------------------------------------------------------- */
 /*      Copy over this pixel of data.                                   */
 /* -------------------------------------------------------------------- */
@@ -239,29 +271,38 @@
 
             if( eDataType == eBufType )
             {
+                /* FIXME: it might be better to call GDALCopyWords to process
+                   all the pixels from the current buffer */
                 if( eRWFlag == GF_Read )
                     memcpy( ((GByte *) pData) + iBufOffset,
                             pabySrcBlock + iSrcOffset, nBandDataSize );
                 else
                     memcpy( pabySrcBlock + iSrcOffset, 
                             ((GByte *) pData) + iBufOffset, nBandDataSize );
+                iBufOffset += nPixelSpace;
             }
             else
             {
-                /* type to type conversion ... ouch, this is expensive way
-                   of handling single words */
-                
+                /* copy all the words for the current image block */
                 if( eRWFlag == GF_Read )
-                    GDALCopyWords( pabySrcBlock + iSrcOffset, eDataType, 0,
-                                   ((GByte *) pData) + iBufOffset, eBufType, 0,
-                                   1 );
+                {
+                    GDALCopyWords( pabySrcBlock + iSrcOffset, eDataType, 
+                                   bytesPerDataPixel * iSrcXInc,
+                                   ((GByte *) pData) + iBufOffset, eBufType, 
+                                   bytesPerBufPixel * iSrcXInc, wordCount );
+                }
                 else
-                    GDALCopyWords( ((GByte *) pData) + iBufOffset, eBufType, 0,
-                                   pabySrcBlock + iSrcOffset, eDataType, 0,
-                                   1 );
+                {
+                    GDALCopyWords( ((GByte *) pData) + iBufOffset, eBufType, 
+                                   bytesPerBufPixel * iSrcXInc,
+                                   pabySrcBlock + iSrcOffset, eDataType, 
+                                   bytesPerDataPixel * iSrcXInc, wordCount );
+                }
+                /* adjust the loop counter to account for copying multiple
+                   words - ugly I know */
+                iBufXOff += (wordCount - 1);
+                iBufOffset += (nPixelSpace * wordCount);
             }
-
-            iBufOffset += nPixelSpace;
         }
     }

Change History (2)

comment:1 Changed 15 years ago by warmerdam

Andrey,

Could you review this patch, and assuming it looks reasonable apply it and
test?  

Thanks,

comment:2 Changed 15 years ago by dron

Patch applied. In my tests ~20% perfomance increasing achieved.
Note: See TracTickets for help on using tickets.