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

[Vladimir Kozlov]

....!!! 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
>>