On Fri, 26 Sep 2025 13:02:57 GMT, Volkan Yazici <[email protected]> wrote:
>> Alan Bateman has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Review feedback >> - Change ciField::initialize_from to use is_mutable_static_final, suggested >> by Vladimir Ivanov > > test/hotspot/jtreg/runtime/jni/mutateFinals/MutateFinalsTest.java line 109: > >> 107: String instanceMethod = list1.get(rand.nextInt(list1.size())); >> 108: String staticMethod = list2.get(rand.nextInt(list2.size())); >> 109: return Stream.of(instanceMethod, staticMethod); > > * What is the rationale for choosing a random `pairOf(instanceMutator, > classMutator)`? > * Does "instance mutator gets followed by a class mutator" have any > particular importance for the tests they are used? > > I was looking at this argument supplier and thinking of > `Collections.shuffle()` over a list containing all method names, preferably, > multiple times. The comment in the test description has ".. to avoid starting a child VM to test every mutation method". I'll see if I can improve this comment to make it clearer, or maybe your comment is that the method source needs this comment too? The context here is that the test is launching a VM with -Xcheck:jni to check that it terminates with a "FATAL ERROR in native method". Doing this for each of 18 cases would be expensive, can be 30+ seconds on macOS systems with debug builds. To keep the test execution down, the test choses one JNI function that attempts to mutate a final instance field, and one JNI function that attempts to mutate a static final field. The ensures the instance + static implementations are tested on each run. There is template expansion, and all functions will get exercised with enough runs. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25115#discussion_r2390255811
