RFR: 8346239: Improve memory efficiency of JimageDiffGenerator

[Archie Cobbs]

On Thu, 19 Dec 2024 18:14:39 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

> Please review this fairly simple change to improve how the `JimageDiffGenerator` works. The original implementation is pretty naive and read all bytes into memory and then compared them. This improved version only reads bytes on a bound buffer into memory compares those bytes and if equal continues on to reading the next bytes (`2k` at most) at a time. Previously it was `2*N` (where `N` is the file size of a file in bytes) part of the JDK. Ouch.
> 
> There is still the off-chance of reading a full file into memory when the generator detects a change, but this shouldn't happen for large files since the total diff should be around `600K` as of today.
> 
> This also reverts changes from [JDK-8344036](https://bugs.openjdk.org/browse/JDK-8344036) other than the `/timeout` change to the test, which is preserved as I think this bump is no longer needed.
> 
> Testing:
> - [x] GHA
> - [x] jlink tests on a fastdebug build with `--with-native-debug-symbols=internal` (so as to have a large libjvm.so).
> - [x] Manual reproducer from the bug which fails with OOM before the patch and passes after
> 
> Thoughts?

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/JimageDiffGenerator.java line 134:

> 132:                 while ((bytesRead1 = is1.read(buf1)) != -1 &&
> 133:                        (bytesRead2 = is2.read(buf2)) != -1) {
> 134:                     if (bytesRead1 != bytesRead2) {

This test seems to assume there won't be any short reads. That might be true in practice, but for defensive programming reasons it would be safer to use `readNBytes()` instead of `read()`.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/JimageDiffGenerator.java line 137:

> 135:                         return false;
> 136:                     }
> 137:                     if (bytesRead1 == buf1.length) {

This can be simplified by using [`Arrays.equals(byte[],int,int,byte[],int,int)`](https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/util/Arrays.html#equals(byte%5B%5D,int,int,byte%5B%5D,int,int)) for both cases.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22835#discussion_r1907828139
PR Review Comment: https://git.openjdk.org/jdk/pull/22835#discussion_r1907830279