On Fri, 3 Apr 2026 03:05:31 GMT, Chris Plummer <[email protected]> wrote:
>> test/jdk/com/sun/jdi/EATests.java line 2411:
>>
>>> 2409: * Test relocking eliminated @ValueBased object.
>>> 2410: */
>>> 2411: class EARelockingObject extends EATestCaseBaseDebugger {
>>
>> The goal of testcase was to specifically lock on the ValueBased object.
>> I think this testacase now doing the same as EARelockingSimple and the best
>> fix would be just to delete this testcase.
>
> You are right. I thought there might be some reason to keep it in, but I see
> now that it basically duplicates EARelockingSimple.
>
> It looks like we are losing some useful testing by not syncing on Integer. In
> #7626, which added the test, the description says:
>
> "Can reproduce a crash by modifying test/jdk/com/sun/jdi/EATests.java and
> using -XX:DiagnoseSyncOnValueBasedClasses=2 with LM_LEGACY or running
> test/jdk/com/sun/jdi/EATests.java with LM_LIGHTWEIGHT/LM_MONITOR and
> -Xlog:monitorinflation=trace."
>
> So we would be losing this testing, which seems like it might be important.
> Eventually it will have to go away when Valhalla moves out of preview. We
> could keep it for now and only run it when PreviewFeatures.isEnabled()
> returns false, but eventually Valhalla will be out of preview and this
> subtest will need to be removed.
>
> @xmas92 Do you think it is useful to keep EARelockingValueBased around until
> Valhalla is out of preview, or should we just get rid of it now?
One other thing to point out is that running with
DiagnoseSyncOnValueBasedClasses=2 was removed by
[JDK-8359437](https://bugs.openjdk.org/browse/JDK-8359437) (@toxaart), but then
for some reason Alex added it back in for
[JDK-8377845](https://bugs.openjdk.org/browse/JDK-8377845). I'm not sure why he
added it back in rather than just remove the comment that was missed by
[JDK-8359437](https://bugs.openjdk.org/browse/JDK-8359437).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30560#discussion_r3034450542
