[15?] RFR (S): 8249192: MonitorInfo stores raw oops across safepoints

Thomas Schatzl thomas.schatzl at oracle.com
Wed Jul 22 08:14:42 UTC 2020


Hi Serguei,

   thanks for your review.

On 22.07.20 04:13, serguei.spitsyn at oracle.com wrote:
> Hi Thomas,
> 
> The fix looks okay to me.
> The 15 fix is identical to 16.

   no :) It is very subtle. As mentioned, in jvmtiEnvBase.cpp in the 
original code, in jdk15 we had:

line 1029:       ResourceMark rm;

in jdk16:

line 1008:       ResourceMark rm(current_thread);

Both lines were ultimately removed with this recent change, but still 
cause merge errors if you port the patch from one version to the other.
Sorry if that has been confusing.

> 
> This file finally was not changed: 
> src/hotspot/share/runtime/vframe_hp.cpp .

This is an artifact of webrev in conjunction with mq because the 
original change had some modifications which were retracted in the -1 
webrev there. There will not be any change in any push for that file.

> Several files need a copyright comment update.

Provided new webrevs with only comment updates below. Did not do new 
testing just for these comment updates though.

> 
> What tests do you run?
> We need at least tier3 to make sure there are no regressions in the 
> JVMTI and JDI tests.

The jdk15 .1 patch has been tested with 1.2k of that LocalsAndOperands 
test with the options that originally reproduced it in 1/100 cases.

hs-tier1-5, no failures at all.

jdk16 has had testing with the .0 patch doing 1.8k of 
LocalsAndOperands.java, tier1-5, and tier8 with JDK-8249676 reinstated 
that earlier caused lots of issues there (and none without).
Since there has been no substantial change in how the patch works, only 
some refactoring, so I think these are still valid.

See the internal comments in the CR for links.

New webrevs:

jdk15:

http://cr.openjdk.java.net/~tschatzl/8249192/webrev.jdk15.2/ (full)
http://cr.openjdk.java.net/~tschatzl/8249192/webrev.jdk15.1_to_2/ (diff)

jdk16:

http://cr.openjdk.java.net/~tschatzl/8249192/webrev.2/ (full)
http://cr.openjdk.java.net/~tschatzl/8249192/webrev.1_to_2/ (diff)

Thanks,
   Thomas


More information about the serviceability-dev mailing list