Request for reviews (M): 6614597: Performance variability in jvm2008 xml.validation

Vladimir Kozlov Vladimir.Kozlov at Sun.COM
Thu Jan 28 11:55:24 PST 2010

Here is new webrev

Tom, you are right. I just need to remove too_many_traps() check
and rely only on counters. We may get an additional deoptimization
and recompilation in the next rare case but I think it is fine.

There is time interval between the method deoptimization and
when interpreter continue execution of the method and increment
total counter for the call bytecode indicating polimorphic case.
During this interval an other thread can trigger compilation and
compiler thread will load MDO which has not updated total counter.
The parser will still see bimorphic case and generate bimorphic code.

And I will use Reason_bimorphic to separate traps.


Tom Rodriguez wrote:
> I assumed you intended to use the state of the counters to distinguish mono from bi, i.e. if the total count is greater than rec1 + rec2 then bimorphic can't work otherwise give it a try.  I agree is might be more clear if Reason_bimorphic were used.  Your changes to make it per bci look fine.
> tom
> On Jan 27, 2010, at 6:45 PM, Vladimir Kozlov wrote:
>> ....!!! I need Reason_bimorphic even with my changes for call counters.
>> Otherwise uncommon trap will not be generated for bimorphic case
>> if the first compilation was with monomorphic case after which
>> we hit class_check trap and recorded it in MDO for this bytecode.
>> I forgot about it and found again during testing of latest changes.
>> And I want to have too_many_traps() check per bytecode instead of current per method.
>> So I am going to place Reason_bimorphic before Reason_unloaded to record it
>> per bytecode. I think the next changes should be enough.
>> Thanks,
>> Vladimir
>> @@ -33,12 +33,15 @@ class Deoptimization : AllStatic {
>>   enum DeoptReason {
>>     Reason_many = -1,             // indicates presence of several reasons
>>     Reason_none = 0,              // indicates absence of a relevant deopt.
>> +    // Next 7 reasons are recorded per bytecode in DataLayout::trap_bits
>>     Reason_null_check,            // saw unexpected null or zero divisor (@bci)
>>     Reason_null_assert,           // saw unexpected non-null or non-zero (@bci)
>>     Reason_range_check,           // saw unexpected array index (@bci)
>>     Reason_class_check,           // saw unexpected object class (@bci)
>>     Reason_array_check,           // saw unexpected array class (aastore @bci)
>>     Reason_intrinsic,             // saw unexpected operand to intrinsic (@bci)
>> +    Reason_bimorphic,             // saw unexpected object class in bimorphic inlining (@bci)
>> +
>>     Reason_unloaded,              // unloaded class or constant pool entry
>>     Reason_uninitialized,         // bad class state (uninitialized)
>>     Reason_unreached,             // code is not reached, compiler
>> @@ -49,7 +52,7 @@ class Deoptimization : AllStatic {
>>     Reason_predicate,             // compiler generated predicate failed
>>     Reason_LIMIT,
>>     // Note:  Keep this enum in sync. with _trap_reason_name.
>> -    Reason_RECORDED_LIMIT = Reason_unloaded   // some are not recorded per bc
>> +    Reason_RECORDED_LIMIT = Reason_bimorphic  // some are not recorded per bc
>>     // Note:  Reason_RECORDED_LIMIT should be < 8 to fit into 3 bits of
>>     // DataLayout::trap_bits.  This dependency is enforced indirectly
>>     // via asserts, to avoid excessive direct header-to-header dependencies.
>> @@ -279,7 +282,7 @@ class Deoptimization : AllStatic {
>>                                        int trap_state);
>>   static bool reason_is_recorded_per_bytecode(DeoptReason reason) {
>> -    return reason > Reason_none && reason < Reason_RECORDED_LIMIT;
>> +    return reason > Reason_none && reason <= Reason_RECORDED_LIMIT;
>>   }
>> Vladimir Kozlov wrote:
>>> Thank you, Tom
>>> Then I will go ahead to implement it.
>>> Vladimir
>>> Tom Rodriguez wrote:
>>>> I think you're right that part of the problem is that even though we maintain 3 counters, the total is supposed to be related to the other two but since they aren't updated atomically they can actually be out of sync by an almost arbitrary amount.  So I think you're suggesting that instead of maintaining rec1, rec2 and total, we should maintain rec1, rec2 and other.  The site count will be the sum of rec1, rec2 and other.  Another nice thing about this would be that we'd only be updating a single counter for the call site instead of normally doing two.  The other way to handle this would be to set TypeProfileWidth to 3.  Either way, if the profile itself could reliably indicate more than 2 receiver types we wouldn't need to have a new Reason.  I like the idea of a solution like this.
>>>> tom
>>>> On Jan 25, 2010, at 10:24 AM, Vladimir Kozlov wrote:
>>>>> Reason_bimorphic will not cover the case when total counter
>>>>> is larger then sum of 2 receivers in real bimorphic case.
>>>>> What I am saying is currently the only reliable information
>>>>> these counters can give us is monomorphic case. We can not
>>>>> relay on them (in current implementation) to distinguish
>>>>> bimorphic case from polimorphic case. This is why Reason_bimorphic
>>>>> helps. If we use counters differently we don't need Reason_bimorphic.
>>>>> Vladimir
>>>>> John Rose wrote:
>>>>>> On Jan 22, 2010, at 10:06 PM, Vladimir Kozlov wrote:
>>>>>>> The third solution (sorry they are still popping up) will be using
>>>>>>> the total counter in MDO only for missing (third receiver) cases.
>>>>>>> It will allow to keep current type profile width and distinguish
>>>>>>> bimorphic case from polimorphic.
>>>>>> I like Tom's idea of mapping Reason_bimorphic to an unrelated reason, such as Reason_range_check.
>>>>>> -- John

More information about the hotspot-compiler-dev mailing list