<div dir="ltr"><div>Great! The PR is available here: <a href="https://github.com/openjdk/jdk/pull/20432">https://github.com/openjdk/jdk/pull/20432</a></div><div><br></div><div>Do you have a few spare cycles to review / sponsor? Happy to make any adjustments, if I've overlooked something.<br></div><div><br></div><div>Take care,</div><div><br></div><div>Daniel</div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Aug 1, 2024 at 8:00 PM Phil Race <<a href="mailto:philip.race@oracle.com">philip.race@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><u></u>

  
  <div>
    <p><a href="https://bugs.openjdk.org/browse/JDK-8337681" target="_blank">https://bugs.openjdk.org/browse/JDK-8337681</a></p>
    <p>-phil.<br>
    </p>
    <div>On 8/1/2024 10:08, Daniel Gredler
      wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr">
        <div>Thanks for the confirmation!</div>
        <div><br>
        </div>
        <div>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).</div>
        <div><br>
        </div>
        <div>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.<br>
        </div>
        <div><br>
        </div>
        <div>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.<br>
        </div>
        <div><br>
        </div>
        <div>Take care,</div>
        <div><br>
        </div>
        <div>Daniel<br>
        </div>
        <div><br>
        </div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">On Wed, Jul 31, 2024 at
          8:13 PM Philip Race <<a href="mailto:philip.race@oracle.com" target="_blank">philip.race@oracle.com</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I
          think yes.<br>
          Whereas JPG grabs the raster for the whole image whereas as
          you noted, <br>
          PNG is looping row by row,<br>
          so there might need to be some updates in the handling of the
          child <br>
          raster needed.<br>
          <br>
          -phil.<br>
          <br>
          <br>
          On 7/31/24 10:02 AM, Daniel Gredler wrote:<br>
          > Hi all,<br>
          ><br>
          > I'd like a quick sanity check on a possible memory
          inefficiency in <br>
          > PNGImageWriter.<br>
          ><br>
          > Some of the other image writers, like JPEGImageWriter, do
          their best <br>
          > to avoid duplicating the image raster + data buffer
          [1]... although it <br>
          > doesn't look like this was always the case [2].<br>
          ><br>
          > The PNGImageWriter, on the other hand, always makes a new
          raster + <br>
          > data buffer copy for each row [3]. It *is* per-row, so
          the data isn't <br>
          > all duplicated in one go, but even on a row-by-row basis,
          it ends up <br>
          > being quite a bit of unnecessary allocation and
          subsequent garbage <br>
          > collection.<br>
          ><br>
          > Can somebody confirm if I'm reading things right, and
          whether <br>
          > PNGImageWriter would benefit from a similar BufferedImage
          + single <br>
          > tile check, such as already exists in JPEGImageWriter?<br>
          ><br>
          > Thanks!<br>
          ><br>
          > Daniel<br>
          ><br>
          > [1] <br>
          > <a href="https://github.com/openjdk/jdk/blob/e4c7850c177899a5da6f5050cb0647a6e1a75d31/src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageWriter.java#L415" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/blob/e4c7850c177899a5da6f5050cb0647a6e1a75d31/src/java.desktop/share/classes/com/sun/imageio/plugins/jpeg/JPEGImageWriter.java#L415</a><br>
          > [2] <a href="https://bugs.openjdk.org/browse/JDK-6266748" rel="noreferrer" target="_blank">https://bugs.openjdk.org/browse/JDK-6266748</a><br>
          > [3] <br>
          > <a href="https://github.com/openjdk/jdk/blob/e4c7850c177899a5da6f5050cb0647a6e1a75d31/src/java.desktop/share/classes/com/sun/imageio/plugins/png/PNGImageWriter.java#L923" rel="noreferrer" target="_blank">https://github.com/openjdk/jdk/blob/e4c7850c177899a5da6f5050cb0647a6e1a75d31/src/java.desktop/share/classes/com/sun/imageio/plugins/png/PNGImageWriter.java#L923</a><br>
          ><br>
          <br>
        </blockquote>
      </div>
    </blockquote>
  </div>

</blockquote></div>