RFR(S): 8174856: [TESTBUG] Missing DefineClass instances

[Dmitry Dmitriev]

Hello Volker,

I think this can be simplified by using isComp() method of Platform 
class from jdk.test.lib package(test/lib/jdk/test/lib/Platform.java). 
Also, I think it is possible to specify VM mode in @requires tag(I think 
it's best solution to exclude test with "-Xcomp" flag), but I need to 
check that tomorrow.

Thank you,
Dmitry

On 15.02.2017 20:32, Volker Simonis wrote:
> Hi,
>
> so here's an alternative version which doesn't execute the tests in -Xcomp mode:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2017/8174856.v1/
>
> Instead of using a wrapper test which seems overly complicated to me
> :) I just use MX to query the command line and return if either
> '-Xcomp' or '-XX:-UseInterpreter' was specified.
>
> Please choose whichever of the two versions you like.
>
> Regards,
> Volker
>
> PS: it would be great if you could run the exact test that failed
> before just in case there will be other problems as well. I've just
> checked that the test now succeed in both, normal and '-Xcomp' mode.
>
>
> On Wed, Feb 15, 2017 at 10:17 AM, David Holmes <david.holmes at oracle.com> wrote:
>> Hi Volker,
>>
>> On 15/02/2017 5:51 PM, Volker Simonis wrote:
>>> On Wed, Feb 15, 2017 at 12:46 AM, David Holmes <david.holmes at oracle.com>
>>> wrote:
>>>> Hi Volker,
>>>>
>>>> Can we just skip the test in -Xcomp mode?
>>>>
>>> That would be fine for me, but how would I detect that? -Xcomp is not
>>> a VM flag, it is specially handled at startup and sets the following
>>> actual flags:
>>>
>>>    case _comp:
>>>      UseInterpreter           = false;
>>>      BackgroundCompilation    = false;
>>>      ClipInlining             = false;
>>>      if (TieredCompilation) {
>>>        Tier3InvokeNotifyFreqLog = 0;
>>>        Tier4InvocationThreshold = 0;
>>>      }
>>>
>>> Do you think it is safe to check for all these flags and skip the test
>>> if they are set correspondingly?
>>
>> A simpler approach would be to run a wrapper test in driver mode, so no VM
>> flags are passed through, and then exec the real test in a VM with minimal
>> flags set.
>>
>>> Why do you actually think we should skip the test in -Xcomp mode? Do
>>> you think it became too complicated/unstable? The good thing about
>>> running it in -Xcomp mode is that it now also checks that we get rid
>>> of the instance classes which are referenced by the nmethods.
>>
>> Xcomp is a stress mode that can sometimes change the normal way things work,
>> so we sometimes end up not actually testing what we intended to test when
>> run in Xcomp mode.
>>
>> In this case I don't know enough about what is happening to evaluate the
>> techniques you have used to deal with Xcomp mode. So I can't review this
>> as-is.
>>
>> Would love to get other opinions on this.
>>
>> Thanks,
>> David
>>
>>
>>> But as I said, I'm open for both solutions. Please let me know what you
>>> think.
>>>
>>> Regards,
>>> Volker
>>>
>>>> David
>>>>
>>>>
>>>> On 15/02/2017 4:31 AM, Volker Simonis wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> can I please have a reviewer and sponsor for the following fix:
>>>>>
>>>>> http://cr.openjdk.java.net/~simonis/webrevs/2017/8174856/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8174856
>>>>>
>>>>> The runtime/Metaspace/DefineClass.java test introduced with 8173743
>>>>> fails if run in '-Xcomp' mode. There are actually two problems we
>>>>> observe:
>>>>>
>>>>> - for the "defineClass" and "defineClassParallel" cases we observe
>>>>> fewer instance classes than expected. This is because in -Xcomp mode
>>>>> the functions get compiled and deoptimized, but because we don't
>>>>> really use the defined classes, they are already removed before our
>>>>> first check. This problem can be easily fixed by inserting the classes
>>>>> into a global static list.
>>>>>
>>>>> - for the "redefineClass" case, if run in -Xcomp mode, the problem is
>>>>> that we get 10 different nmethods for each version of the class
>>>>> redefinition, and these nmethods keep the instance classes alive, even
>>>>> after their activation has been removed from the stack. The problem
>>>>> can be solved by manually deoptimizing the methods and subsequent
>>>>> sweeping of the code cache. Notice that this has to be done at least
>>>>> three times, because during every sweep process, the nmethods first
>>>>> transition from "non-entrant" to "zombie" before they are finally
>>>>> cleaned.
>>>>>
>>>>> Thank you and best regards,
>>>>> Volker
>>>>>