RFR: 8364927: Add @requires annotation to TestReclaimStringsLeaksMemory.java [v2]
jonghoonpark
duke at openjdk.org
Wed Sep 3 11:28:46 UTC 2025
On Wed, 3 Sep 2025 09:47:51 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> Thank you for your feedback.
>>
>> Then, would modifying the code as shown below be acceptable?
>>
>>
>> /*
>> * @test TestReclaimStringsLeaksMemory
>> * ...
>> * @run driver/timeout=480 gc.stress.TestReclaimStringsLeaksMemory
>> */
>>
>> /*
>> * @test id=Serial
>> * ...
>> * @run driver/timeout=480 gc.stress.TestReclaimStringsLeaksMemory -XX:+UseSerialGC
>> */
>>
>> /*
>> * @test id=Parallel
>> * ...
>> * @run driver/timeout=480 gc.stress.TestReclaimStringsLeaksMemory -XX:+UseParallelGC
>> */
>>
>> /*
>> * @test id=G1
>> * ...
>> * @run driver/timeout=480 gc.stress.TestReclaimStringsLeaksMemory -XX:+UseG1GC
>> */
>
> Do you really need the first one? It will cause us to run G1 twice. If you really need it I would prefer if you used id= and gave it a proper id. Otherwise the run will just say TestReclaimStringsLeaksMemory#id0
I included that part because it was already present in the original code.
I thought it was there to test the case where a GC other than SerialGC, ParallelGC, or G1GC might be used as the default.
However, after reviewing your feedback, I realized that the other tests clearly focus only on the specific GCs that are actually required.
Then, would it be better to remove the first test and add tests for other GCs later only if they turn out to be necessary?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26766#discussion_r2318666522
More information about the hotspot-gc-dev
mailing list