[Gdal-dev] [RFC] RasterIO speedup patch

Tue Feb 18 09:36:02 EST 2003

Resend... Frank, you didn't reply to this email and it has a question in 
that I need an answer to...

Thanks
Tim

----- Forwarded by Tim Beckmann/NMD/CONT/USGS/DOI on 02/18/2003 08:35 AM 
-----

Tim Beckmann
02/05/2003 05:23 PM

        To:     gdal-dev at remotesensing.org
        cc: 
        Subject:        Re: [Gdal-dev] [RFC] RasterIO speedup patch

Frank,

Okay, I put it in bugzilla.  It is #281.

I still need to clean up the other changes a bit before I submit them. One 
question for you.  When GDALCopyWords does data type conversions, it uses 
memcpy for copying out a single data element at a time.  The only reason I 
can think of why this would be done is that you're worried about being 
given data buffers that aren't properly aligned in memory for platforms 
that have such limitations.  Is this correct?  The next set of changes 
assumes memory alignment is correct.  I can either try using memcpy and 
see if it impacts speed in my changes, or I can make it one more special 
case that the changes I've done are only used when memory alignment 
constraints are met.  Actually, I can't think of a case where memory 
wouldn't be correctly aligned when going through RasterIO.  But I suppose 
someone could end up with incorrectly aligned buffers when calling 
GDALCopyWords directly (but for efficiency maybe it should just error out 
in that case and the driver should be fixed).

I agree that 1:1 IO should be as efficient as possible.  I won't be 
worrying about the down/up sampling speed right now since it is a feature 
we won't be using in the near future in LAS.  But the data type 
conversions are something we depend on the I/O layer to do currently.

A regression test is a good idea.  We have a large one for LAS.  But right 
now it only exercises our own LAS driver when we're using GDAL.  That will 
change in the future (probably distant).

BTW, I should have mentioned that I work with April Shimitz and Lowell 
Johnson who have filled you in a little on what we're doing.

Thanks
Tim

--------------------------------------------------------------------------
Tim Beckmann               tbeckman at usgs.gov
Software Project Lead
SAIC
EROS Data Center, Sioux Falls, SD 57198
605-594-2521    Phone
605-594-6940    Fax

Frank Warmerdam <warmerdam at pobox.com>
Sent by: gdal-dev-admin at remotesensing.org
02/05/2003 04:43 PM
Please respond to gdal-dev

        To:     gdal-dev at remotesensing.org
        cc: 
        Subject:        Re: [Gdal-dev] [RFC] RasterIO speedup patch

Tim Beckmann wrote:
> Frank and the rest of the list,
> 
> We've been working to incorporate GDAL into our software.  One drawback 
> we've found so far is that the performance of gdal when it needs to do 
> data type conversions is extremely slow.  Here is a patch that I've 
> generated that seems to work to improve the performance.  It works by 
> changing the GDALRasterBand::IRasterIO to attempt to convert more than a 

> single pixel for every call to GDALCopyWords.  For the testing I've 
done, 
> it seems to work well.  I'm not sure if I've covered all the possible 
> combinations though.  There are still cases where it falls back to 
calling 
> GDALCopyWords once for each pixel.
> 
> Some preliminary timings show that the runtime to convert a 100 MB 
Float32 
> image to Byte data took 49 seconds with the original code, but only 27 
> seconds with the patch below.  This is using our unreleased LAS driver 
on 
> an SGI Origin 2000 with 250 MHz processors.
> 
> Any comments?

Tim,

Based on a quick inspection the code looks OK.  Could you submit this
patch into bugzilla as an enhancement request?  I will have Andrey apply
and test it.

> Oh yeah, once for the record - I really dislike the naming convention 
used 
> for variables in the routine.  It makes it much harder to understand the 

> code.  >  Also, some of the variable names could use some comments to 
explain
> what they do (I'd add them, but I'm not 100% sure).  Example: the 
> nSrcPixelOffset parameter to GDALCopyWords.  If I understand it 
correctly, 
> it isn't an offset, it is the value to stride through the data with.

Well, I think of it as the offset from the beginning of one pixel to the
beginning of the next.  I would agree of course that more internal
documentation would be helpful.

> I also have another set of changes that cut the time nearly in half 
> again...  But one step at a time.

I look forward to receiving them.

There is clearly lots of room for improvement in GDAL, at least in
cases where up/down sampling is done and/or when data type conversion is
done.   I have tried to ensure that the 1:1 IO logic (especially on
block boundaries) is efficient.

I would also add that I think I need to write some regression tests that
exercise lots of the cases so that when we tune it we can have greater
confidence that we haven't broken anything.

Best regards,

-- 
---------------------------------------+--------------------------------------
I set the clouds in motion - turn up   | Frank Warmerdam, 
warmerdam at pobox.com
light and sound - activate the windows | http://pobox.com/~warmerdam
and watch the world go round - Rush    | Geospatial Programmer for Rent

_______________________________________________
Gdal-dev mailing list
Gdal-dev at remotesensing.org
http://remotesensing.org/mailman/listinfo/gdal-dev