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

[David Holmes]

On 16/02/2017 8:06 PM, Volker Simonis wrote:
> On Wed, Feb 15, 2017 at 6:46 PM, Dmitry Dmitriev
> <dmitry.dmitriev at oracle.com> wrote:
>> 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.
>>
>
> Hi Dmitry,
>
> thanks for you comments. I think you are right and I agree that a
> @requires tag is the simplest and most effective way of excluding
> these test in -Xcomp mode. I use @requires vm.compMode != "Xcomp".
> Please find the new webrev here:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2017/8174856.v2/

Perfect! :) Reviewed!

Thank you Dmitry for pointing out the different @requires options 
available to check.

Thanks,
David
------

> By the way, is there an up to date list of all the available require
> names? I only found
> http://openjdk.java.net/jtreg/tag-spec.html#requires_names but it
> doesn't contain vm.compMode != "Xcomp" for example.
>
> Regards,
> Volker
>
>> 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
>>>>>>>
>>