Request for reviews (S): 6923002: assert(false,"this call site should not be polymorphic")
Vladimir Kozlov
Vladimir.Kozlov at Sun.COM
Thu Feb 4 16:40:06 PST 2010
Thank you, Tom
Vladimir
Tom Rodriguez wrote:
> On Feb 4, 2010, at 4:07 PM, Vladimir Kozlov wrote:
>
>> First, all our profiling logic works with empty rows in any slot.
>> I verified it since I also thought about repacking but it is not necessary.
>> We do sorting when building profiling info (ciCallProfile) for compilation.
>
> You're right. The code will all deal with this correctly.
>
>> Also I intentionally want to make it monomorphic since it allow to have
>> more accurate profiling information because there was execution phase change
>> (klasses were unloaded). And if the site still polymorphic MDO will be updated
>> to reflect it. But it could be the case that the site becomes only bimorphic.
>> Then keeping total count not 0 will be wrong.
>> Even if we use monomorphic (when it is not) for compilation we will only have
>> trap, deoptimization and recompile again with updated MDO (after executing method
>> in Interpreter).
>
> Agreed. I'm ok with the changes then. One more typo s/Cleare/Clear/.
>
> tom
>
>> Thanks,
>> Vladimir
>>
>>
>> Tom Rodriguez wrote:
>>> Why is clearing the count the right solution? I understand that it makes the code work but it's clearing actual history too so that the call site doesn't reflect what was actually invoked. A call site could suddenly look monomorphic when in fact it isn't. I guess I would have expected that the count of the cleared row would be added to the count. Also I think the logic that clears the slots should repack the entries so that none of the earlier slots are empty.
>>> tom
>>> On Feb 4, 2010, at 9:43 AM, Vladimir Kozlov wrote:
>>>> http://cr.openjdk.java.net/~kvn/6923002/webrev
>>>>
>>>> Fixed 6923002: assert(false,"this call site should not be polymorphic")
>>>>
>>>> Problem:
>>>> After the 6614597 changes C2 expects that the MDO total count at the virtual
>>>> call site indicates only polimorphic case and the assert was added for that.
>>>> But ReceiverTypeData::follow_weak_refs() may clear a receiver information
>>>> in MDO leaving the data at strange state and causing assert.
>>>>
>>>> Solution:
>>>> Clear also the total count when a receiver information is cleared.
>>>> An additional receiver, if it exists, will be recorded in the cleaned row
>>>> during next execution of the call site.
>>>> Also add method name for the assert output and record in logs a second
>>>> receiver even for polimorphic case.
>>>>
>>>> Reviewed by:
>>>>
>>>> Fix verified (y/n): y, test
>>>>
>>>> Other testing:
>>>> JPRT
>>>>
>
More information about the hotspot-compiler-dev
mailing list