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

[Stefan Karlsson]

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/20150413/f6ce62fa/attachment.htm>