RFR (M) 8139551: Scalability problem with redefinition - multiple code cache walks

[serguei.spitsyn at oracle.com]

On 2/4/19 15:05, David Holmes wrote:
> On 5/02/2019 8:30 am, serguei.spitsyn at oracle.com wrote:
>> Hi Coleen,
>>
>> Looks good in general.
>>
>> Just a couple minor comments below.
>>
>> I hope, you will update the copyright years before the push.
>>
>>
>> http://cr.openjdk.java.net/~coleenp/2019/8139551.01/webrev/test/hotspot/jtreg/runtime/RedefineTests/TestMultipleClasses.java.html 
>>
>>
>>    Nice test!
>>
>>    37 import java.lang.reflect.*;
>>    38 import jdk.test.lib.compiler.InMemoryJavaCompiler;
>>    39 import java.lang.instrument.*;
>>
>>    The imports above can be re-ordered.
>>
>>    44         return new String("public class B" + count + " {" +
>>    45                 "   public static void compiledMethod() { " +
>>    46                 "       try{" +
>>    47                 " Thread.currentThread().sleep(1); " +
>>    48                 "       } catch(InterruptedException ie) {" +
>>    49                 "       }" +
>>    50                 "   }" +
>>    51                 "}");
>>
>>    Not clear, why sleep is needed here.
>
> Presumably just to give it some code and consume some time.

Some print statement would be more simple and useful.
But it is really minor. :)

Thanks,
Serguei


> Nit: But it should just be:
>
> Thread.sleep(1)
>
> not
>
> Thread.currentThread().sleep(1);
>
> sleep is a static method and as with all Thread static methods always 
> applies to the current thread.
>
> David
>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 2/4/19 12:34, serguei.spitsyn at oracle.com wrote:
>>> Added the serviceability-dev back.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 2/4/19 07:18, coleen.phillimore at oracle.com wrote:
>>>>
>>>> open webrev at 
>>>> http://cr.openjdk.java.net/~coleenp/2019/8139551.01/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8139551
>>>>
>>>> The links.
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 2/4/19 10:08 AM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: Walk code cache and deoptimize once per redefinition.*
>>>>>
>>>>> *This change touches the AOT and code cache code.  I tried to 
>>>>> describe it in the comments in the RFE.  Basically, for 
>>>>> redefinition, we walk the code cache per class redefined in order 
>>>>> to find evol_method dependencies, then deoptimize if we find them, 
>>>>> and then walk the code cache again to mark the deoptimized 
>>>>> nmethods as not_entrant.
>>>>>
>>>>> I replaced this with only marking any nmethods for the class's 
>>>>> methods to deoptimize (following InstanceKlass::_methods), also 
>>>>> marking aot methods dependent on the class, and then doing the 
>>>>> code cache walk to follow the evol_method dependencies at the end 
>>>>> of redefinition for all the classes. The new code uses the 
>>>>> Method::is_old() flag rather than comparing each method in the 
>>>>> InstanceKlass.  Then deoptimization and marking not_entrant is 
>>>>> done once at the end of the redefinition as well.
>>>>>
>>>>> Tested with tier1-6, all the redefinition tests locally:
>>>>>
>>>>> make test TEST=open/test/hotspot/jtreg/vmTestbase/nsk/jvmti 
>>>>> >&jvmti.out
>>>>> make test TEST=open/test/hotspot/jtreg/vmTestbase/nsk/jdi >&jdi.out
>>>>> make test TEST=open/test/hotspot/jtreg/runtime/RedefineTests 
>>>>> >&redefine.out
>>>>> make test TEST=open/test/jdk/java/lang/instrument >&instrument.out
>>>>>
>>>>> new test, and JMH test (see CR for results).
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>>
>>>>>
>>>>
>>>
>>