Request for reviews (M): 6614597: Performance variability in jvm2008 xml.validation
Tom Rodriguez
Thomas.Rodriguez at Sun.COM
Thu Jan 28 09:55:56 PST 2010
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