RFR(S): 8005079: fix LogCompilation for incremental inlining

[Vladimir Kozlov]

Hi Roland,


On 3/26/14 2:06 AM, Roland Westrelin wrote:
> Hi Vladimir,
>
> Thanks for taking another look.
>
>> Need CICountOSR check, shift compile_id by 32 and use bit 31 to indicate OSR when needed (I doubt we get unique() with bit 31 set):
>>
>>      jlong id = ((jlong)unique()) + ((jlong)compile_id())<<32 + (CICountOSR && is_osr_compilation() ? 1<<31 : 0);
>>
>> Otherwise good.
>
> Can you explain why this is better?

CICountOSR is off by default, osr compilations have compile_id in the same sequence as normal compilations. Only when 
CICountOSR is on its compile_id can match one of normal compilation id.

And your formula is incorrect because produced id for compile_id = 2 will match id for osr_compile_id = 1, for example.

If you want to keep low 32 bit for unique() you can shift more:

((jlong)compile_id())<<33 + (CICountOSR && is_osr_compilation() ? 1<<32 : 0);

Regards,
Vladimir

> I don’t see why the test for CICountOSR is needed. Without the test for CICountOSR, the generated id is unique as well, whether CICountOSR is on or off.
> The most important part is that the id be unique within a compilation otherwise the LogCompilation tool can be confused. I played it extra safe by keeping the lower part for unique() which is what makes the id unique within a compilation. Why put the compile id in its own 32 bit half? What problem does it solve?
>
> Roland.
>
>
>>
>> Thanks,
>> Vladimir
>>
>> On 3/25/14 7:00 AM, Roland Westrelin wrote:
>>> Hi Vladimir,
>>>
>>>>>> It is good.
>>>>>
>>>>> Thanks for the review.
>>>>>
>>>>>> One thing you may need to change is C->log_inline_id((jlong)this).
>>>>>> Address is not good id when you look on log file and it could be the same between compilations.
>>>>>> I think some combination of compile_id<<shift+(adjustment for OSR when CICountOSR=true)+(unique node number at time when cg was created or other sequential number).
>>>>>
>>>>> I don’t understand why it matters. The id is only used within a single compilation by the LogCompilation tool to correctly match a call site at which we stopped inlining with a call site at which we start to inline again later on.
>>>>
>>>> It will be confusing when you search in a log file for such id and it matches ids from other compilations.
>>>
>>> I followed your suggestion:
>>> http://cr.openjdk.java.net/~roland/8005079/webrev.04/
>>>
>>> Roland.
>>>
>>>>
>>>> Vladimir
>>>>
>>>>>
>>>>> Roland.
>>>>>
>>>>>> Thanks,
>>>>>> vladimir
>>>>>>
>>>>>> On 3/21/14 2:04 AM, Roland Westrelin wrote:
>>>>>>>>> http://cr.openjdk.java.net/~roland/8005079/webrev.02/
>>>>>>>>
>>>>>>>> In order for equals to be correct you have to do an instanceof check.
>>>>>>>
>>>>>>> Ok.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~roland/8005079/webrev.03/
>>>>>>>
>>>>>>> Roland.
>>>>>>>
>>>>>
>>>
>