On 2018-11-29 17:40, Aleksey Shipilev wrote:
On 11/29/18 5:26 PM, Per Liden wrote:
Hi,
On 11/29/18 4:48 PM, Aleksey Shipilev wrote:
On 11/29/18 4:34 PM, Per Liden wrote:
On 11/29/18 4:17 PM, Aleksey Shipilev wrote:
On 11/29/18 4:07 PM, Per Liden wrote:
There's no support for SA heap walking when using ZGC. Hence the tests for this
should not execute
when ZGC is enabled.
Bug: https://bugs.openjdk.java.net/browse/JDK-8214484
Webrev: http://cr.openjdk.java.net/~pliden/8214484/webrev.0
Looks good.
Stylistic: why not "!vm.gc.Z"?
That has a slightly different meaning. vm.gc.Z will be true if -XX:+UseZGC is an
"acceptable"
option. For example, ZGC is supported by the build and no other GC was
explicitly chosen. vm.gc will
be "Z" if the test was explicitly executed with -XX:+UseZGC.
So if we used !vm.gc.Z on a build with ZGC support, then the test would never
execute, unless you
also explicitly specified what GC to use.
Yeah, I it's messy... I hope I got the details right there... ;)
Mmm. That's not what we see with TestFullGCCount test, for example, which has
pre-existing check for
CMS, and our new check for Shenandoah:
* @requires !(vm.gc.ConcMarkSweep & vm.opt.ExplicitGCInvokesConcurrent ==
true)
* @requires !(vm.gc.Shenandoah & vm.opt.ExplicitGCInvokesConcurrent == true)
Those @requires tags look wrong to me. With the above, the test will not be
executed if you _only_
supply -XX:+ExplicitGCInvokesConcurrent (and use the default GC). It will work
as intended if you
instead use:
@requires !(vm.gc == "ConcMarkSweep" & vm.opt.ExplicitGCInvokesConcurrent)
@requires !(vm.gc == "Shenandoah" & vm.opt.ExplicitGCInvokesConcurrent)
Right?
Damn. Right. What a mess! Your current patch looks good then.
Thanks for reviewing!
Btw, I don't think anyone would object if someone proposed a better and
less confusing scheme to deal with the vm.gc stuff.
/Per
