RFR (S): 8007988: PrintInlining output is inconsistent with incremental inlining

[Vladimir Ivanov]

Looks good to me, considering it'll be cleaned up as a separate change.

Best regards,
Vladimir Ivanov

On 4/4/14 11:29 AM, Roland Westrelin wrote:
> Hi Vladimir,
>
>>>>> Roland, I think current and suggested implementation is a mess. It is
>>>>> based on how we produced PrintInlining output before. We patched all
>>>>> places where we did print inlining before.
>>>>>
>>>>> Replay tool uses InlineTree info to dump and reload inlining
>>>>> information. Can PrintInlining use it too (by adding needed information
>>>>> into it)? Please, explain if we can't do that because it may not work
>>>>> for Replay too and we have to fix it.
>>>>
>>>> Answer: because InlineTree does not record inlining failures.
>>>>
>>>> Roland, I will go through your changes and look how we can do it more elegant. Essentially we need to keep records of all call sites and their inlining state. We can order them later before printing by bci and inline depth (the order is messed up by later inline).
>>>
>>> This change fixes the existing mess but doesn’t make any messier, does it? Can’t I simply push this so that PrintInlining is more accurate and file a bug to clean it up later if someone feels like it?
>>
>> Okay, I agree.
>
> Thanks. So I can consider it reviewed by you?
> Anyone else for this review?
>
> Roland.
>
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> Roland.
>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 3/18/14 11:38 AM, Roland Westrelin wrote:
>>>>>> The PrintInlining output with incremental inlining can be a mess.
>>>>>> Currently it can print 2 lines for a single call site with conflicting
>>>>>> messages and when that doesn’t happen sometimes report a failed
>>>>>> inlining with a wrong reason.
>>>>>>
>>>>>> PrintInlining messages are currently stored in a list of buffers.
>>>>>> Every new message is appended to the current buffer in the list. When
>>>>>> a call site is enqueued for late inlining then we allocate a new
>>>>>> buffer for the subsequent messages and enqueue it after the current
>>>>>> buffer. The new buffer becomes the current buffer. When we inline the
>>>>>> call site, we can then insert a new buffer for new messages in between
>>>>>> the two 2 buffers and keep inlining messages ordered. This doesn’t
>>>>>> work well because, when we enqueue a late inlining call site, we’ve
>>>>>> already appended the PrintInlining messages for this call site to the
>>>>>> current buffer. When we try to inline that late inlining call site, we
>>>>>> want to be able to change those messages (it may have failed the first
>>>>>> time around but succeeds now or it may have reported success but have
>>>>>> been delayed and now we are out of nodes so we can’t do the inlining).
>>>>>> To achieve this, this change appends PrintInlining messages for the
>>>>>> current call site to a staging
>>>>> buffer, then if the call site is a late inlining call site, we allocate
>>>>> a new buffer and add the messages to that buffer. This way we can go
>>>>> back later and update the messages.
>>>>>>
>>>>>> This should also solve the assert failures with PrintInlining that are
>>>>>> sometimes seen: https://bugs.openjdk.java.net/browse/JDK-8028274
>>>>>>
>>>>>> http://cr.openjdk.java.net/~roland/8007988/webrev.00/
>>>>>>
>>>>>> Roland.
>>>>>>
>>>
>