Hi Sherman,

On 13/03/2018 5:13 AM, Xueming Shen wrote:
On 3/11/18, 7:48 PM, David Holmes wrote:
Hi Sherman,


Thanks for working on this and adding the new functionality to OutputAnalyzer, but note that:

+   private static final String jvmwarningmsg =
+       "Java HotSpot\\(TM\\) 64-Bit Server VM warning:.*";

is Oracle JDK specific and 64-bit and server VM specific! Other tests use:

Pattern.compile(".*VM warning.*")

to exclude all VM warnings. Sorry that wasn't clear from previous discussions.

Hi David,

I was trying to be conservative to only filter out the "vm warnings" particularly for this test failure. The webrev has been updated as suggested to filter out any line with
"VM warning".

The issue isn't about being conservative. You need to exclude the warnings no matter what VM the test is executed on. The original code would only pass on the Oracle JDK, and 64-bit Server VM. If run on OpenJDK the warning has the form:

OpenJDK 64-Bit Server VM warning: ...

If run on 32-bit there's no "64-bit". If run on Client VM there's no "Server".


Verified with the warning msg turned on as
http://cr.openjdk.java.net/~sherman/8196748/webrev.hs/src/hotspot/share/runtime/arguments.cpp.sdiff.html
and ran the jtreg with -XX:+FastTLABRefill.

That will not hit all cases as it requires the -XX:+FastTLABRefill to be passed through to all JVMs launched by tests. The original problem occurred with unconditional warnings that did not depend on a particular flag being specified on the command-line. All of those cases have now been fixed however so just reenabling the message doesn't achieve anything.

The reason I did not try to turn "PrintSafepointStatisticsCount" back on (to reproduce) is that it was
not included in the changeset published in JDK-8196739 as
http://hg.openjdk.java.net/jdk/jdk/rev/74be5b4ed152

It appears the reason I failed to reproduce LeadingGarbage.java is the test in my repo does not forward the vm options (I'm not sure why the test forwards those vm options in your repo). I have updated the test

I think there is a misunderstanding here. It doesn't matter what flag generates the warning - I modified PrintSafepointStatisticsCount as a convenience because the flags that originally triggered this problem no longer exist as flags (and so can't trigger the problem). The flag that generates the warning does not get specified as a VM argument to jtreg and does not get passed through to any other tools or VMs launched (there's no difference between our repos). That is the key point. In its original form (before the relevant flags were actually removed) we experience unconditional warnings from the VM and every execution of the VM (whether java, javac, jar, javap etc) is affected. Your test only triggered a warning due to the use of the FastTLABRefill flag and only the VM's that are explicitly passed that flag will encounter the warning. That excluded the VM (jar?) that actually triggers the test failure.

case to use the new test.lib (it was using the "old" jdk.testlibrary), and forward the vmoptions (this one
should have nothing to do with old libs or new libs though)

Right. Good to modernize the test, but not directly related to current issue.

Now the failure is reproducible and has been fixed with new method in OutputAnalyzer.java.

webrev: http://cr.openjdk.java.net/~sherman/8196748/webrev

test/jdk/tools/jar/modularJar/Basic.java

+ } else if (line.contains("Java HotSpot(TM) 64-Bit Server VM warning:")) {
+                         continue;  // ignore server vm warning see#8196748

That needs to be any VM warning as discussed.

---

test/lib/process/OutputAnalyzer.java

I'm not sure if "replace" is the right strategy here, if stderr may be very large. ?? The approach in JDK-8194067 was to see if the output lines matched the warning (though there are a couple of deficiencies in the way it did it).

Otherwise, my only minor concern is the assumption that the VM warnings will always be on stderr. That is the default, but it can be changed.

Thanks,
David

(arguments.cpp will not be put back, just to help reproduce)

Thanks,
Sherman

Reply via email to