On Wed, 1 May 2024 16:10:06 GMT, Justin Lu <j...@openjdk.org> 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