On Fri, 14 Mar 2025 23:21:59 GMT, Leonid Mesnik <[email protected]> wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments from @erikj79 and @lmesnik
>
> make/RunTests.gmk line 738:
>
>> 736: $1_AOT_JDK_CACHE := $$($1_TEST_SUPPORT_DIR)/aot/jdk.aotcache
>> 737: $1_AOT_JDK_LOG := $$($1_TEST_SUPPORT_DIR)/aot/TestSetupAOT.log
>> 738: $1_AOT_JDK_OUTPUT_DIR := $$($1_TEST_SUPPORT_DIR)/aot
>
> Better to move it before line 735, and reuse in other definitions instead of
> $$($1_TEST_SUPPORT_DIR)/aot
Fixed.
> test/setup_aot/TestSetupAOT.java line 147:
>
>> 145:
>> 146:
>> 147: static void streamOps(String args[]) {
>
> Can you please add a comment with explanation of purpose of 'streamOps'
> method.
I renamed the method to `invokedynamicTests()` and added comments.
> test/setup_aot/TestSetupAOT.java line 201:
>
>> 199: String CSCC = "string" + s + "string" + c;
>> 200:
>> 201: long l = System.currentTimeMillis();
>
> the l should be logged and allow to be defined using property.
> So any testing could be repeated.
> long l = Long.getLong("test.aot.seed", System.currentTimeMillis());
I changed it to not depend on system time. Now the program should be
deterministic.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996506289
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996506299
PR Review Comment: https://git.openjdk.org/jdk/pull/24067#discussion_r1996506308
