RFR: JDK-8077265 Modify assert to help debug JDK-8068448

[Stefan Karlsson]

On 2015-04-14 17:30, Eric Caspole wrote:
> Hi, here's a new webrev with the new assert in psOldGen as a function -
> I capture only the covered_region in a local as it directly relates to 
> the problem.
>
> http://cr.openjdk.java.net/~ecaspole/JDK-8077265/02/webrev/

Looks good.

StefanK

>
> Passes JPRT.
> Thanks,
> Eric
>
>
> On 4/13/2015 4:39 PM, Stefan Karlsson wrote:
>> On 2015-04-13 17:52, Eric Caspole wrote:
>>> Thinking about it over the weekend, I really do want some new 
>>> asserts in psOldGen because it should catch the problem in a 
>>> different/interesting way.
>>> I am happy to back out the  changes in psPromotionLAB.cpp and hpp, 
>>> since that part already catches the problem.
>>>
>>> Stefan, do you want to this change to use the macro in psOldGen as I 
>>> was doing before or refactor into a new function?
>>
>> I think I'd prefer a new function to get rid of the setup of the 
>> local __variable_name__ variables. We would lose the line number of 
>> the call site, if we replace the macro with a function, but the stack 
>> trace in the hs_err file tell us where we came from, so I think 
>> that's OK.
>>
>> Thanks,
>> StefanK
>>
>>
>>> Thanks,
>>> Eric
>>>
>>>
>>> On 4/10/2015 9:19 AM, Stefan Karlsson wrote:
>>>> On 2015-04-10 15:15, Eric Caspole wrote:
>>>>> I don't want to make sweeping changes to try to debug what seems 
>>>>> like a race.
>>>>> If you dont like the macro in psOldGen I will remove it and just 
>>>>> go on with the original one.
>>>>
>>>> Sounds good to me.
>>>>
>>>> StefanK
>>>>
>>>>> Eric
>>>>>
>>>>>
>>>>> On 4/10/2015 4:43 AM, Stefan Karlsson wrote:
>>>>>> On 2015-04-09 22:17, Eric Caspole wrote:
>>>>>>> Hi everybody,
>>>>>>> I updated this so the psOldGen part use a macro as Stefan 
>>>>>>> suggested.
>>>>>>> The assert in psPromotionLAB.hpp is allocating out of an already 
>>>>>>> allocated PLAB, so I don't think that one will ever be hit but I 
>>>>>>> want it there just in case.
>>>>>>> And as Jesper suggested I made the message more helpful in the 
>>>>>>> original place.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~ecaspole/JDK-8077265/01/webrev/
>>>>>>
>>>>>> I had hoped you would use the new define for all assert, but 
>>>>>> you've only used it for two out of four. If you're not going to 
>>>>>> use the same macro for all usages, it doesn't seem worth having.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~ecaspole/JDK-8077265/01/webrev/src/share/vm/gc_implementation/parallelScavenge/psOldGen.hpp.udiff.html 
>>>>>>
>>>>>>
>>>>>> It seems to me that we could refactor allocate_noexpand and 
>>>>>> cas_allocate_noexpand to use a common function to do the assert 
>>>>>> and update the start array. That way you could get rid of the 
>>>>>> macro you added.
>>>>>>
>>>>>> Thanks,
>>>>>> StefanK
>>>>>>
>>>>>>>
>>>>>>> Passes JPRT.
>>>>>>> Thanks,
>>>>>>> Eric
>>>>>>>
>>>>>>> On 4/9/2015 10:01 AM, Stefan Karlsson wrote:
>>>>>>>> Hi Eric,
>>>>>>>>
>>>>>>>> On 2015-04-09 15:19, Eric Caspole wrote:
>>>>>>>>> HI everybody,
>>>>>>>>> Here is a webrev to add more asserts related to debugging 
>>>>>>>>> JDK-8068448. Beyond capturing more info in the original 
>>>>>>>>> assert, after looking at another core I added more asserts to 
>>>>>>>>> make sure there is no other place where old gen allocations 
>>>>>>>>> would overrun the start array.
>>>>>>>>
>>>>>>>> Why didn't these two new asserts get the same, more 
>>>>>>>> informative, error message as the first assert you changed? 
>>>>>>>> Maybe you could extract the check out to a helper macro that 
>>>>>>>> prints the relevant information?
>>>>>>>>
>>>>>>>> Another point that Bengt mentioned yesterday, is that we don't 
>>>>>>>> really need to print the old_gen part of the assert. It's 
>>>>>>>> already printed in the hs_err file.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> StefanK
>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~ecaspole/JDK-8077265/00/webrev/
>>>>>>>>>
>>>>>>>>> Passes JPRT.
>>>>>>>>> Thanks,
>>>>>>>>> Eric
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150416/3ce621d2/attachment.htm>