search

harrydb / go

12 stars 8 forks source link

6.34 Speedup in decodeRawRGB with a big-read and fewer slices. #1

Closed jcmdev0 closed 11 years ago

jcmdev0 commented 11 years ago

Thanks for putting this library up on github.

I am using go to load some ~80MB pnm files output from a scanner and things were running a little more slowly (13.8s for 10 decode iterations). After a little profiling, I came up with this change for decodeRawRGB. It lowers the number of system calls by reading the RGB data into the start of the RGBA pixel array and then repacks. I initially used copy(m.Pix[i_4:i_4+3], m.Pix[i_3:i_3+3]), but the memmove overhead associated with the slices was still high in the profiling (took ~8s for 10 decodes). Unrolling the slice copy brought it down to ~2.2s for 10 iterations.

Please let me know if you have any concerns.

Thanks, John

harrydb commented 11 years ago

Hey!

Thanks for using it :-) I'll take a look at your code.

Cheers, Harry

harrydb commented 11 years ago

Looks good, I merged it. I think I'll apply this in other places as well (notably decodeRawRGB64).

Cheers, Harry

harrydb commented 11 years ago

Hey John,

there was a subtle bug in the RGBA unpack code. Because of the order in which the bytes were unpacked some where overwritten because of the reversed loop. Reversing the order fixes this, see commit 770d1e8aee. I will upload some code for testing and benchmarks soon.

Harry

jcmdev0 commented 11 years ago

Harry,

Good catch! All of my images are white in the corner and I didn't think to check.

Thanks, John

On Fri, Aug 16, 2013 at 3:41 PM, Harry de Boer notifications@github.comwrote:

Hey John,

there was a subtle bug in the RGBA unpack code. Because of the order in which the bytes were unpacked some where overwritten because of the reversed loop. Reversing the order fixes this, see commit 770d1e8https://github.com/harrydb/go/commit/770d1e8aee. I will upload some code for testing and benchmarks soon.

Harry

— Reply to this email directly or view it on GitHubhttps://github.com/harrydb/go/pull/1#issuecomment-22798605 .