RFR(S): 8026694: New type profiling points break compilation replay

[Vladimir Kozlov]

Ship it.

Vladimir

On 5/14/14 9:16 AM, Roland Westrelin wrote:
>> src/share/vm/ci/ciReplay.cpp:
>>
>> !   Klass**   _classes_handles;
>> !   Method**  _methods_handles;
>>
>> Remove the _handles suffix because it’s not that case anymore.
>
> I will make that change.
>
>> Otherwise this looks good.
>
> Thanks for the review, Christian.
>
> Following, the fix for "8032463: VirtualDispatch test timeout with DeoptimizeALot”, the change below is also required:
>
> diff --git a/src/share/vm/runtime/vmStructs.cpp b/src/share/vm/runtime/vmStructs.cpp
> --- a/src/share/vm/runtime/vmStructs.cpp
> +++ b/src/share/vm/runtime/vmStructs.cpp
> @@ -2503,6 +2503,7 @@
>     declare_constant(Deoptimization::Reason_speculate_class_check)          \
>     declare_constant(Deoptimization::Reason_speculate_null_check)           \
>     declare_constant(Deoptimization::Reason_rtm_state_change)               \
> +  declare_constant(Deoptimization::Reason_tenured)                        \
>     declare_constant(Deoptimization::Reason_LIMIT)                          \
>     declare_constant(Deoptimization::Reason_RECORDED_LIMIT)                 \
>                                                                             \
> No objection that I push this small fix with the rest?
>
> Roland.
>
>
>>
>> On Apr 18, 2014, at 2:33 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:
>>
>>>
>>> Here a new webrev with the SA support:
>>>
>>> http://cr.openjdk.java.net/~roland/8026694/webrev.01/
>>>
>>> The ciReplay jtreg tests now pass.
>>>
>>> Roland.
>>>
>>>
>>> On Apr 15, 2014, at 11:56 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>
>>>> Roland,
>>>>
>>>> Can you look on compiler/ciReplay/TestSA.sh failure? Is it because of additional metadata?  Will this your change fix it? Do you need to update SA too?
>>>>
>>>> I got next running latest promoted jdk9 fastdebug VM with Tiered off:
>>>>
>>>> WARNING: replay.txt from SA != replay.txt from VM:
>>>>
>>>> < # ciMetadata0 @ sun.jvm.hotspot.oops.TypeArrayKlass at 0x0000000100000030
>>>> < # Unknown ci type 0xfffffd7ed7ecc5a0
>>>> ...
>>>> < # ciMetadata98 @ sun.jvm.hotspot.oops.Method at 0xfffffd7ffccf9da0
>>>> 504a396,403
>>>>> ciMethodData java/lang/invoke/MethodHandle <clinit> ()V 1 0 orig 304 104 155 245 215 126 253 255 255 0 0 0 0 0 0 0 0 240 46 198 252 127 253 255 255 208 2 0 0
>>>>
>>>> And:
>>>>
>>>> Exception in thread "main" java.lang.InternalError: missing reason for 17
>>>>        at sun.jvm.hotspot.oops.MethodData.initialize(MethodData.java:185)
>>>>        at sun.jvm.hotspot.oops.MethodData.access$000(MethodData.java:36)
>>>>        at sun.jvm.hotspot.oops.MethodData$1.update(MethodData.java:126)
>>>>        at sun.jvm.hotspot.runtime.VM.registerVMInitializedObserver(VM.java:394)
>>>>        at sun.jvm.hotspot.oops.MethodData.<clinit>(MethodData.java:124)
>>>>
>>>> thanks,
>>>> Vladimir
>>>>
>>>> On 4/15/14 11:50 AM, Vladimir Kozlov wrote:
>>>>> On 4/15/14 11:21 AM, Roland Westrelin wrote:
>>>>>>>>>>> http://cr.openjdk.java.net/~roland/8026694/webrev.00/
>>>>>>>>>>>
>>>>>>>>>>> If we want to still be able to read replay files generated before
>>>>>>>>>>> this change, the assert:
>>>>>>>>>>>
>>>>>>>>>>> src/share/vm/ci/ciReplay.cpp
>>>>>>>>>>> 1120       assert(m->_data_size + m->_extra_data_size ==
>>>>>>>>>>> rec->_data_length * (int)sizeof(rec->_data[0]), "must agree”);
>>>>>>>>>>>
>>>>>>>>>>> needs to removed or changed (extra data from the MDO is not
>>>>>>>>>>> currently dumped).
>>>>>>>>>
>>>>>>>>> Are you asking what to do with the assert? Can you detect if replay
>>>>>>>>> file has extra data?
>>>>>>>>
>>>>>>>> I can change the assert to:
>>>>>>>>
>>>>>>>>       assert(m->_data_size + m->_extra_data_size ==
>>>>>>>> rec->_data_length * (int)sizeof(rec->_data[0]) ||
>>>>>>>>              m->_data_size == rec->_data_length *
>>>>>>>> (int)sizeof(rec->_data[0]), "must agree”);
>>>>>>>>
>>>>>>>> and then it covers both cases. There’s no real need to detect
>>>>>>>> whether the replay file has extra data.
>>>>>>>
>>>>>>> Agree.
>>>>>>
>>>>>> Thanks Vladimir. Can I consider this reviewed?
>>>>>
>>>>> Yes.
>>>>>
>>>>> Vladimir
>>>>>
>>>>>>
>>>>>> Roland.
>>>>>>
>>>
>>
>