PNG image writing: memory inefficiency?

[Daniel Gredler]

Great! The PR is available here: https://github.com/openjdk/jdk/pull/20432

Do you have a few spare cycles to review / sponsor? Happy to make any
adjustments, if I've overlooked something.

Take care,

Daniel



On Thu, Aug 1, 2024 at 8:00 PM Phil Race <philip.race at oracle.com> wrote:

> https://bugs.openjdk.org/browse/JDK-8337681
>
> -phil.
> On 8/1/2024 10:08, Daniel Gredler wrote:
>
> Thanks for the confirmation!
>
> I went ahead and made some measurements, and the improvement possibilities
> really are quite good -- in a local test PNGImageWriter.write( ) and
> callees were initially showing up as an allocation hotspot (~400k objects /
> ~65 MB allocated), but after some optimizations I'm seeing much better
> numbers (~400 objects / ~740 KB allocated).
>
> I also spent some time creating regression tests for different
> combinations of alpha pre-multiplied vs. not, BufferedImage vs. other image
> types, untiled vs. single-tile vs. multi-tile, custom source bands vs.
> none, etc.
>
> I've filed a bug (internal review ID 9077379), but do not have a bug
> number yet. Once I have a bug number I'll submit a PR for review.
>
> Take care,
>
> Daniel
>
>
> On Wed, Jul 31, 2024 at 8:13 PM Philip Race <philip.race at oracle.com>
> wrote:
>
>> I think yes.
>> Whereas JPG grabs the raster for the whole image whereas as you noted,
>> PNG is looping row by row,
>> so there might need to be some updates in the handling of the child
>> raster needed.
>>
>> -phil.
>>
>>
>> On 7/31/24 10:02 AM, Daniel Gredler wrote:
>> > Hi all,
>> >
>> > I'd like a quick sanity check on a possible memory inefficiency in
>> > PNGImageWriter.
>> >
>> > Some of the other image writers, like JPEGImageWriter, do their best
>> > to avoid duplicating the image raster + data buffer [1]... although it
>> > doesn't look like this was always the case [2].
>> >
>> > The PNGImageWriter, on the other hand, always makes a new raster +
>> > data buffer copy for each row [3]. It *is* per-row, so the data isn't
>> > all duplicated in one go, but even on a row-by-row basis, it ends up
>> > being quite a bit of unnecessary allocation and subsequent garbage
>> > collection.
>> >
>> > Can somebody confirm if I'm reading things right, and whether
>> > PNGImageWriter would benefit from a similar BufferedImage + single
>> > tile check, such as already exists in JPEGImageWriter?
>> >
>> > Thanks!
>> >
>> > Daniel
>> >
>> > [1]
>> >
>> https://github.com/openjdk/jdk/blob/e4c7850c177899a5da6f5050cb0647a6e1a75d31/src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageWriter.java#L415
>> > [2] https://bugs.openjdk.org/browse/JDK-6266748
>> > [3]
>> >
>> https://github.com/openjdk/jdk/blob/e4c7850c177899a5da6f5050cb0647a6e1a75d31/src/java.desktop/share/classes/com/sun/imageio/plugins/png/PNGImageWriter.java#L923
>> >
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/client-libs-dev/attachments/20240801/6f676396/attachment.htm>