Openj9: sanity.system MauveMultiThreadLoadTest DateFormat.equals

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk11_j9_sanity.system_ppc64_aix_Release/21
MauveMultiThreadLoadTest_0

LT  FAIL: gnu.testlet.java.text.DateFormat.equals (number 0)
LT  FAIL: gnu.testlet.java.text.DateFormat.equals (number 1)

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk11_j9_sanity.system_ppc64_aix_Nightly/325
MauveMultiThreadLoadTest_0

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_ppc64le_linux_xl_Personal_mauveLoadTest/4
MauveMultiThreadLoadTest_special_3
variation: Mode107
JVM_OPTIONS: -Xgcpolicy:optthruput -Xdebug -Xrunjdwp:transport=dt_socket,address=8888,server=y,onthrow=no.pkg.foo,launch=echo -Xjit:count=0 -Xnocompressedrefs

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_x86-64_linux_Nightly_mauveLoadTest/28
MauveMultiThreadLoadTest_special_25
variation: Mode610-OSRG
JVM_OPTIONS: -Xcompressedrefs -Xjit:enableOSR,enableOSROnGuardFailure,count=1,disableAsyncCompilation -Xgcpolicy:gencon

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_x86-64_mac_Nightly_mauveLoadTest/28
MauveMultiThreadLoadTest_special_26
variation: Mode612-OSRG
JVM_OPTIONS: -Xcompressedrefs -Xgcpolicy:gencon -Xjit:enableOSR,enableOSROnGuardFailure,count=1,disableAsyncCompilation

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk14_j9_sanity.system_ppc64_aix_Nightly/96
MauveMultiThreadLoadTest_0

[pshipton]

Tentatively assigning to jit given the intermittent nature of the failure.

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_x86-64_mac_Nightly_mauveLoadTest/31
MauveMultiThreadLoadTest_special_26
variation: Mode612-OSRG
JVM_OPTIONS: -Xcompressedrefs -Xgcpolicy:gencon -Xjit:enableOSR,enableOSROnGuardFailure,count=1,disableAsyncCompilation

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_x86-64_mac_Nightly_mauveLoadTest/32
MauveMultiThreadLoadTest_special_26

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_s390x_linux_xl_Personal_mauveLoadTest/5
MauveMultiThreadLoadTest_special_8
variation: Mode122
JVM_OPTIONS: -Xgcpolicy:optavgpause -Xjit:count=0,optlevel=warm,gcOnResolve,rtResolve -Xnocompressedrefs

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_sanity.system_ppc64_aix_Nightly/406
MauveMultiThreadLoadTest_0

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_ppc64_aix_Nightly_mauveLoadTest/40
MauveMultiThreadLoadTest_special_25

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_x86-64_linux_Nightly_mauveLoadTest/35
MauveMultiThreadLoadTest_special_26

[DanHeidinga]

@JasonFengJ9 Can you take a look at the test and validate it's valid to run this in the multi-threaded way it's being run?

[JasonFengJ9]

@DanHeidinga The investigation shows that it is not valid to run this test in the multi-threaded way as per following details.

The test code snippet in question are:

public class equals implements Testlet
{
  /**
   * Runs the test using the specified harness.
   * @param harness  the test harness (<code>null</code> not allowed).
   */
  public void test(TestHarness harness)
  {
    DateFormat f1 = new SimpleDateFormat("yyyy");
    DateFormat f2 = new SimpleDateFormat("yyyy");
    harness.check(f1.equals(f2));                  // check 1  ---> failed
    harness.check(f2.equals(f1));                  // check 2  ---> failed 

SimpleDateFormat(pattern) is equivalent to calling SimpleDateFormat(pattern, Locale.getDefault(Locale.Category.FORMAT)).

If there is another thread calling setDefault(Locale.Category, Locale) with a different Locale between those two SimpleDateFormat constructors, they might be not equal. Following are a few such tests:

./gnu/testlet/java/util/GregorianCalendar/setFirstDayOfWeek.java:    Locale.setDefault(Locale.GERMANY);
./gnu/testlet/java/util/Currency/Japan.java:    Locale.setDefault(TEST_LOCALE);
./gnu/testlet/java/text/DecimalFormat/format.java:    Locale.setDefault(Locale.UK);
./gnu/testlet/javax/swing/plaf/basic/BasicFileChooserUI/installStrings.java:    Locale.setDefault(Locale.FRANCE);
...

So this test is problematic in a multi-thread environment in which current JVM default Locale could be changed by another thread.
Since this is a legacy test package and no easy way to modify the source, suggest for individual test exclusion or running this test suite in a single thread mode.

fyi @Mesbah-Alam

[DanHeidinga]

@JasonFengJ9 thanks for tracking this down.

Since this is a legacy test package and no easy way to modify the source, suggest for individual test exclusion or running this test suite in a single thread mode.

The test is run in the OpenJ9 jenkins so the source should be accessible and modifyable. @Mesbah-Alam can you link to the source for this test?

[Mesbah-Alam]

The test is part of the third party Mauve test package that gets downloaded from its public site by the systemtest.getDependency build. So, I think instead of trying to modify the source, we should exclude this test from the multi-threaded target and only run it in the single-threaded target-- as @JasonFengJ9 suggested above.

[JasonFengJ9]

It appears the mauve.jar is built from source download via cvs.root=:pserver:[email protected]:/cvs/mauve, is this correct?
if so, we might consider contribute a fix to the mauve repository.
The fix for this failure is to supply a specified local instead of using JVM default which could be modified by another thread.

As per https://www.sourceware.org/mauve/contribute.html, _Contributing to Mauve is fairly easy -- we're pretty generous with cvs write access, and we don't require copyright assignments or anything like that. We do ask that you not have read Sun's source; we are a cleanroom test suite and we're concerned about contamination._
@Mesbah-Alam what do you think?

fyi @smlambert @llxia

[Mesbah-Alam]

I don't think we have ever tried to commit anything to the mauve project before, so I am not aware of the process and how long they take to respond / merge a change log entry (that's the format in which they expect a contribution to be). It'd be a good thing to try. @JasonFengJ9 - Do you already have the change to submit?

[pshipton]

I don't think there is anything wrong with the tests, it's the way we're running them multi-threaded which is causing the problems. Some of the tests have side affects which affect other tests. We've had similar problems before with other mauve tests.

From the multi-threaded testing, we should either exclude all the tests which call Locale.setDefault(), or all the tests that use the default locale. I expect the former is easier to track down, Jason already provided the list.

[JasonFengJ9]

Following code snippet with specified local can work in a multi-threaded environment.

DateFormat f1 = new SimpleDateFormat("yyyy", Locale.US);  --> specify a Locale instead of JVM default
DateFormat f2 = new SimpleDateFormat("yyyy", Locale.US);  --> specify a Locale instead of JVM default

In current Mauve test, the two DateFormat instances won't equal if JVM default local is changed between two constructors.

DateFormat f1 = new SimpleDateFormat("yyyy");
Locale.setDefault(Locale.GERMANY);
DateFormat f2 = new SimpleDateFormat("yyyy");
System.out.println(f1.equals(f2)); // check 1 ---> not equal, and the test fails
System.out.println(f2.equals(f1)); // check 2 ---> not equal, and the test fails

With the proposed modification, following two DateFormat instances still equal even the JVM default local is changed between two constructors.

DateFormat f1 = new SimpleDateFormat("yyyy", Locale.US);
Locale.setDefault(Locale.GERMANY);      
DateFormat f2 = new SimpleDateFormat("yyyy", Locale.US);
System.out.println(f1.equals(f2)); // check 1 ---> still equal, and the test passes
System.out.println(f2.equals(f1)); // check 2 ---> still equal, and the test passes

I agree that excluding the tests or running in single thread mode is an easier workaround.

[Mesbah-Alam]

From the multi-threaded testing, we should either exclude all the tests which call Locale.setDefault(), or all the tests that use the default locale. I expect the former is easier to track down, Jason already provided the list.

I agree. I will create a PR for the excludes.

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_ppc64le_linux_Release_mauveLoadTest/2
MauveMultiThreadLoadTest_special_25

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_x86-32_windows_Release_mauveLoadTest/2
MauveMultiThreadLoadTest_special_8
variation: Mode122
JVM_OPTIONS: -Xgcpolicy:optavgpause -Xjit:count=0,optlevel=warm,gcOnResolve,rtResolve -Xnocompressedrefs

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_ppc64le_linux_xl_Personal_mauveLoadTest/6
MauveMultiThreadLoadTest_special_23
variation: Mode107-OSRG
JVM_OPTIONS: -Xgcpolicy:optthruput -Xdebug -Xrunjdwp:transport=dt_socket,address=8888,server=y,onthrow=no.pkg.foo,launch=echo -Xjit:enableOSR,enableOSROnGuardFailure,count=1,disableAsyncCompilation

[pshipton]

Another failure
https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_ppc64_aix_Nightly_mauveLoadTest/50
MauveMultiThreadLoadTest_special_26

The test is now running with -Xtrace:iprint=mt,trigger=method{java/util/Locale.setDefault*,jstacktrace}, but I don't see any stack trace in the output.

@Mesbah-Alam can we trigger a core dump when one of the tests fail?

[pshipton]

I don't expect this will get resolved in the 0.21 release, moving forward.

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_ppc64_aix_Nightly_mauveLoadTest/59
MauveMultiThreadLoadTest_special_13

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_x86-64_linux_Nightly_mauveLoadTest/54
MauveMultiThreadLoadTest_special_21

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_x86-64_mac_Nightly_mauveLoadTest/55
MauveMultiThreadLoadTest_special_26

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_ppc64_aix_Release_mauveLoadTest/4
MauveMultiThreadLoadTest_special_18

[pshipton]

@Mesbah-Alam the place where https://github.com/AdoptOpenJDK/stf/pull/82 throws MauveTestFailureException isn't when the failure occurs, it's when the failures are reported. The exception needs to happen at the time the first failure is discovered, and the DateFormat objects are on the stack.

Stack:

void net/adoptopenjdk/loadTest/LoadTestRunner$2.reportFailure(long, java.io.ByteArrayOutputStream, net.adoptopenjdk.loadTest.adaptors.AdaptorInterface, net.adoptopenjdk.loadTest.SuiteData, int)  source: LoadTestRunner.java:289
void net/adoptopenjdk/loadTest/LoadTestRunner$2.run()  source: LoadTestRunner.java:215
void java/util/concurrent/ThreadPoolExecutor.runWorker(java.util.concurrent.ThreadPoolExecutor$Worker)  source: ThreadPoolExecutor.java:1149
void java/util/concurrent/ThreadPoolExecutor$Worker.run()  source: ThreadPoolExecutor.java:624
 void java/lang/Thread.run()  source: Thread.java:823

[Mesbah-Alam]

The exception needs to happen at the time the first failure is discovered, and the DateFormat objects are on the stack

Do you want the exception to be added in the Mauve test code itself? This means we'd have to request the change to be added by the Mauve proect.

[pshipton]

Is there no other way to hook into the failure? Do you have a link to the test code which is causing the failure and now it handles the failure.

[Mesbah-Alam]

Throwing of the exception was added into the STF wrapper that runs Mauve test loads, here: https://github.com/AdoptOpenJDK/stf/blob/ad1a2360c73ec638ccedc45abbc80d75f6ee2001/stf.load/src/stf.load/net/adoptopenjdk/loadTest/LoadTestRunner.java#L289

[pshipton]

I'm not looking for a link to what was added and doesn't work, I'm looking for a link to the actual test which is failing, and how the framework handles that failure.

[Mesbah-Alam]

The test class that fails is gnu.testlet.java.text.DateFormat.equals

The source can be seen by downloading mauve.jar from systemtest.getDependency job, unzipping it, and looking into gnu/testlet/java/text/DateFormat folder.

The framework code may be found under Harness.java and RunProcess.java under main mauve folder.

[pshipton]

See here where the MauveAdaptor creates an instance of SingleTestHarness.

https://github.com/AdoptOpenJDK/stf/tree/master/stf.load/src/stf.load/net/adoptopenjdk/loadTest/adaptors/MauveAdaptor.java#L56

What you need to do is create a subclass of gnu.testlet.SingleTestHarness, and change the MauveAdaptor to create an instance of this subclass and use it instead. In the subclass override the implementation of debug(String). It can call super.debug(String), and then throw the MauveTestFailureException. You can catch the exception (https://github.com/AdoptOpenJDK/stf/tree/master/stf.load/src/stf.load/net/adoptopenjdk/loadTest/adaptors/MauveAdaptor.java#L75) and proceed.

[pshipton]

Actually, creating the subclass of SingleTestHarness isn't required. You can check the testResult at the following location and throw the MauveTestFailureException.

https://github.com/AdoptOpenJDK/stf/tree/master/stf.load/src/stf.load/net/adoptopenjdk/loadTest/reporting/ExecutionTracker.java#L98

[pshipton]

https://ci.eclipse.org/openj9/job/Test_openjdk8_j9_special.system_s390x_linux_xl_Nightly_mauveLoadTest/72

@Mesbah-Alam here is another failure, but we didn't get any core dump. The -Xdump command https://github.com/AdoptOpenJDK/openjdk-systemtest/pull/356 doesn't specify the correct package name. It specifies
net/adoptopenjdk/loadTest/MauveTestFailureException, but the Exception is
net.adoptopenjdk.loadTest.reporting.MauveTestFailureException

https://github.com/AdoptOpenJDK/openjdk-systemtest/pull/356

[Mesbah-Alam]

MauveTestFailureException class was refactored into a new package in https://github.com/AdoptOpenJDK/stf/pull/83/files#diff-ad5f317c88f122ae7ecf2be60b18f61e , but the reference was not updated in https://github.com/AdoptOpenJDK/openjdk-systemtest/pull/356/files#diff-a7b21278a26c58dc97240147769b0135 - my mistake. Fixing it now.

[pshipton]

We got a core! I located the two objects being compared and looked at them manually. The difference is in the calendar TimeZone objects. One has GMT and the other Etc/UTC. So I think the problem is not with something calling Locale.setDefault() but calling TimeZone.setDefault().

@Mesbah-Alam can you please check the other mauve tests for callers of TimeZone.setDefault().

[pshipton]

Fixed via https://github.com/AdoptOpenJDK/openjdk-systemtest/pull/358