On Thu, 19 Dec 2024 18:14:39 GMT, Severin Gehwolf <sgehw...@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?

Thanks for the reviews. I'll follow up with those comments/fixes in a separate 
PR.

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

PR Comment: https://git.openjdk.org/jdk/pull/22835#issuecomment-2579597940

Reply via email to