RFR: 8337681: PNGImageWriter uses much more memory than necessary [v2]

[Daniel Gredler]

On Tue, 3 Sep 2024 17:07:46 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> Daniel Gredler has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - JDK-8337681: add tests for TYPE_4BYTE_ABGR
>>  - JDK-8337681: add tests for TYPE_INT_RGB
>
> src/java.desktop/share/classes/com/sun/imageio/plugins/png/PNGImageWriter.java line 924:
> 
>> 922:         for (int row = minY + yOffset; row < minY + height; row += ySkip) {
>> 923:             Raster ras;
>> 924:             if (image instanceof BufferedImage bi) {
> 
> Do we really need to run these two blocks in the loop for each iteration?

Hi Sergey! The current logic does make it very clear where and how `ras` is initialized, which is nice. The top two initialization blocks (for `BufferedImage`s and tiled images) could in theory be hoisted outside the loop, but the third one cannot (and we might have to jump through some hoops to guarantee to the compiler that the variable is initialized if we start initializing it sometimes in one place and sometimes in another).

AFAICT the `BufferedImage.getRaster()` call is not doing any work (just providing a reference to an instance variable), but the `getTile(...)` call might be doing some extra work, depending on the specific instance type (but in any event will not be copying raster data, which was by far the biggest waste here).

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20432#discussion_r1743656653