image: optimize Image.At().RGBA() #15759
Comments
|
To @nigeltao for triage. |
|
Yeah, encoding arbitrary images can be faster, although I wouldn't do it this way. Ideally, we shouldn't call any functions at all per-pixel, inside the inner loop. We want zero calls, rather than one (the AtFunc) or two (At and then RGBA). Instead, I think you'd want something like a ToRGBA64(dst []color.RGBA64, sRect image.Rectangle) (dRect image.Rectangle) method (although that name isn't great) to do the conversion in batches, similar to the way that e.g. the io.Reader interface deals with []byte and not byte, and a compress/zlib.Reader implements io.Reader and not io.ByteReader. In hindsight, maybe the core image.Image method should have been this instead of At, but we can't change that until Go 2. For completeness, a separate approach (type switching on the source image type) is taken by the image/draw and golang.org/x/image/draw pacages. |
I agree
Do you mean a How do you determine the size of Is This approach requires to allocate a large slice of It also requires to rewrite all "image encode" functions in a non trivial way.
Of course. It should be relatively easy to write a function It doesn't change the
This approach requires to write a specific code for each image type. |
|
I mean that ToRGBA64 should be a method that encoders look for. We can't add any methods to the image.Image interface in Go 1.x, though. The sRect and dRect are meant to be 2-D equivalent to the io.ReaderAt interface. Yes, it will mean allocating, but I still suspect that it will be significantly faster than 1 function call per pixel. |
|
This has come up again in https://go-review.googlesource.com/c/go/+/72370 "image/draw: reduce drawPaletted allocations for special source cases" I'm still not sure what the right answer is in the long term, but just noting an idea in the short term, before I forget it: We could add AtDotRGBA methods to all the concrete image types, such as *image.RGBA. m.AtDotRGBA(x, y) is conceptually equivalent to m.At(x, y).RGBA(), except that it avoid the allocation and indirection of the color.Color interface. The AtDotRGBA name is admittedly awkward but tries to avoid ambiguity with the existing method names: RGBAAt is already taken and has different semantics. An alternative is to add a RGBA64At(x, y) method to all of the concrete image types, not just *image.RGBA64. The concept is very similar to AtDotRGBA but it returns one color.RGBA64 value instead of four uint32 values. (Yes, in hindsight, color.Color's RGBA method should have been named RGBA64, and return one color.RGBA64 value instead of four uint32 values. But that ship has sailed until at least Go 2.0, if not beyond). We can't change the image.Image interface, for backwards compatibility, but encoders (or image/draw) can sniff for this AtDotRGBA or RGBA64At method. The subtle difference between this idea and the OP is that the OP is asking for a function or method that returns a function typed variable, and e.g. the jpeg encoder would call that function inside the inner loop. This idea adds a method that returns (4 x uint32) or returns (1 x color.RGBA64). The difference in the jpeg package would be versus where the atter type and NewAtter's body is is: To repeat, I'm not saying that this idea is a good idea (or a better idea than the OP), but I still wanted to record it. Long term, I'd still like some sort of bulk-operations method, analgous to io.Reader instead of io.ByteReader, but it might not be an either/or decision and it might be possible to do both. |
|
Change https://golang.org/cl/72370 mentions this issue: |
drawPaletted has to discover R,G,B,A color values of each source image pixel in a given rectangle. Doing that by calling image.Image.At() method returning color.Color interface is quite taxing allocation-wise since interface values go through heap. Introduce special cases for some concrete source types by fetching color values using type-specific methods. name old time/op new time/op delta Paletted-4 7.62ms ± 4% 3.72ms ± 3% -51.20% (p=0.008 n=5+5) name old alloc/op new alloc/op delta Paletted-4 480kB ± 0% 0kB ± 0% -99.99% (p=0.000 n=4+5) name old allocs/op new allocs/op delta Paletted-4 120k ± 0% 0k ± 0% -100.00% (p=0.008 n=5+5) Updates #15759. Change-Id: I0ce1770ff600ac80599541aaad4c2c826855c8fb Reviewed-on: https://go-review.googlesource.com/72370 Reviewed-by: Nigel Tao <nigeltao@golang.org>
|
|
benchmark old ns/op new ns/op delta Benchmark_Load-8 49097440 10258184 -79.11% benchmark old allocs new allocs delta Benchmark_Load-8 616333 55 -99.99% benchmark old bytes new bytes delta Benchmark_Load-8 2937995 1013548 -65.50% Ref: golang/go#24499 golang/go#15759
In the Go standard library (and x/image), we can see several calls that look like
Image.At().RGBA().These calls mostly happen in y/x loops.
They are slow because they allocate memory.
I have written a library that speeds up
Image.At().RGBA().It is available on https://github.com/pierrre/imageutil
If you call https://godoc.org/github.com/pierrre/imageutil#NewAtFunc with an image, it returns a https://godoc.org/github.com/pierrre/imageutil#AtFunc
A
AtFuncreturns the RGBA value (uint32) for a given pixel (x/y).A call to
NewAtFunc()allocates memory, but it only happens 1 time per image.A call to a
AtFuncdoesn't allocate memory (most of the time).There is also
NewSetFunc()that helps to set RGBA values to an image.It works the same way.
My code is just a copy/paste of the Go standard library, but organized differently.
Benchmarks!
I've written benchmarks for "encode" functions.
It only tests blank images, but it will give you an idea.
Results:
My library should also speed up "copy" function (from an image type to another).
The modifications in the standard library and x/image are really straightforward:
\.At\(.*\)\.RGBA\(\)to a call to aAtFuncNewAtFunc()before a "y/x" loopThe
image/jpeg.toYCbCr()is tricky.You need to call
NewAtFuncinimage/jpeg.encoder.writeSOS()and pass theAtFunctotoYCbCr().I also had to move my library to the standard library, because the go tool complained about bad imports...
As you can see: https://github.com/pierrre/imageutil
my code is tested and it returns the same results as the standard library.
The text was updated successfully, but these errors were encountered: