On Wed, 1 May 2024 16:10:06 GMT, Justin Lu <[email protected]> wrote:
> Please review this PR which converts _TestEncodingDecodingLength.java_ into a
> whitebox test which allows for testing to be done without memory usage issues.
>
> Currently, the test requires about ~2.75 `Integer.MAX_VALUE` sized byte
> arrays worth of memory. (2 for the initial array allocation, .75 for the
> target array in `decode()`). While the `-Xms6g -Xmx8g` options should address
> this, there have been intermittent memory issues, as the underlying machine
> machine may be running other tests simultaneously.
>
> By converting this test to a white-box test not only does it get rid of
> memory issues, but it also gets rid of the need to decode 2GB of data 3
> times. The change is done using reflection to test the private visibility
> methods `encodedOutLength` and `decodedOutLength`, which the public `encode`
> and `decode` overloaded methods call respectively.
Thank you for taking this on Justin and converting to a junit test as part of
your updates.
Overall it looks good on an initial pass.
Please see a few suggestions for your consideration
test/jdk/java/util/Base64/TestEncodingDecodingLength.java line 47:
> 45: public class TestEncodingDecodingLength {
> 46:
> 47: private static final int size = Integer.MAX_VALUE - 8;
Perhaps consider size-> SIZE and maybe tweak the name and add an additional
comment to its purpose
test/jdk/java/util/Base64/TestEncodingDecodingLength.java line 67:
> 65:
> 66: // OOME case
> 67: try {
Perhaps make this a separate test case so that the IAE test fails the OOME
still runs
-------------
PR Review: https://git.openjdk.org/jdk/pull/19036#pullrequestreview-2033944705
PR Review Comment: https://git.openjdk.org/jdk/pull/19036#discussion_r1586493998
PR Review Comment: https://git.openjdk.org/jdk/pull/19036#discussion_r1586499085
