On Fri, 20 Dec 2024 07:53:07 GMT, Matthias Baesken <mbaes...@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?
>
> Hi Severin, I added it to our build/test queue.

Thanks for the review @MBaesken!

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

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

Reply via email to