RFR (S) 6909265: assert(_OnDeck != Self->_MutexEvent, "invariant") with -XX:+PrintMallocFree

[coleen.phillimore at oracle.com]

On 2/2/18 8:32 AM, David Holmes wrote:
> On 2/02/2018 11:26 PM, coleen.phillimore at oracle.com wrote:
>> On 2/2/18 12:08 AM, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> On 2/02/2018 2:24 PM, coleen.phillimore at oracle.com wrote:
>>>>
>>>> Okay, here is PrintMallocFree and PrintMalloc removed. tier1,2 
>>>> testing is in progress.
>>>
>>> So the difference now is that even more of the logging has been 
>>> removed, leaving only a couple of special cases?
>>
>> Right.
>>>
>>>>
>>>> I changed two things in addition to removing this tracing.
>>>>
>>>> 1. Arena::operator new(nothrow) calls AllocateHeap to be consistent 
>>>> with it's throw alternative.  I suspect it called os::malloc() from 
>>>> before AllocateHeap had the flag AllocFailStrategy::RETURN_NULL.
>>>
>>> Okay. Took me a little while to follow it through. :)
>>>
>>>> 2.  I have log tags malloc/free as tags to log_warning when we get 
>>>> a memory stomping error and tracing MallocCatchPtr which is better 
>>>> than tty->print alternative in our UL world.
>>>
>>> It seems odd to me to have log tags "malloc" and "free" but not have 
>>> them actually log mallocs or frees! Shouldn't these just be 
>>> unconditional warnings? Afterall if I get a memory stomp I'd like to 
>>> know about it now - not sure what would prompt me to enable special 
>>> logging just to see that. ??
>>
>> You get log_warning by default.
>
> Ah! I forgot about that. Okay. Ignore me.

Thanks, David.  Thanks for your comments and review on this really old bug!

Coleen

>
> Thanks,
> David
>
>> I think it's an improvement and we'd like to convert all tty->print 
>> to logging, and this seems a perfect case.  It reports fatal() at the 
>> end.
>>
>> thanks,
>> Coleen
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/6909265.02/webrev
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-6909265
>>>
>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>
>>>> On 2/1/18 6:04 PM, David Holmes wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> I have no experience using PrintMallocFree. Unless someone wants 
>>>>> to champion its existance then I'd suggest just getting rid of it. 
>>>>> We have/had a few flags that were presumably useful while 
>>>>> debugging/devoloping something and which are left in "just in 
>>>>> case" but then never get used.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>> On 1/02/2018 11:37 PM, coleen.phillimore at oracle.com wrote:
>>>>>>
>>>>>>
>>>>>> On 1/31/18 10:36 PM, David Holmes wrote:
>>>>>>> Hi Coleen,
>>>>>>>
>>>>>>> On 1/02/2018 1:01 PM, coleen.phillimore at oracle.com wrote:
>>>>>>>> Summary: Convert to logging without thread locking
>>>>>>>>
>>>>>>>> There are two options (-XX:+PrintMallocFree and 
>>>>>>>> -XX:+PrintMalloc) to print the calls and memory returned in 
>>>>>>>> malloc and free calls in the vm. I converted the first one to 
>>>>>>>> Unified Logging which doesn't crash getting the tty lock in the 
>>>>>>>> Thread destructor and removed the latter.  I don't see the 
>>>>>>>> usefulness of this logging honestly, so if the opinion is to 
>>>>>>>> remove this logging, I'd be happy to do so. NMT seems much more 
>>>>>>>> useful.
>>>>>>>
>>>>>>> I'm not sure about this one. Arguably some interesting 
>>>>>>> malloc/frees occur before log configuration.
>>>>>>
>>>>>> They also occurred before tty initialization and afterward too.
>>>>>>>
>>>>>>> That aside, given you always seem to do:
>>>>>>>
>>>>>>> log_is_enabled(Trace, malloc, free)
>>>>>>>
>>>>>>> which requires
>>>>>>>
>>>>>>> -Xlog:malloc+free=trace
>>>>>>>
>>>>>>> would it make more sense to define a single tag eg mallocfree or 
>>>>>>> nativemem or ???
>>>>>>>
>>>>>>
>>>>>> It's an opinion question.  I picked this to match the option and 
>>>>>> assume one would want to see both mallocs and frees.  I like the 
>>>>>> composition of small tags that have meaning.  Then again, I could 
>>>>>> change it if you have an opinion about this.
>>>>>>> Or, as you say, just drop this altogether. Is it useful for when 
>>>>>>> debugging NMT?
>>>>>>>
>>>>>>
>>>>>> I doubt it's useful for debugging NMT.  It might have been useful 
>>>>>> once but I don't know why.  What's your opinion?
>>>>>>
>>>>>> thanks,
>>>>>> Coleen
>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>> Tested with NMT and tier1 tests, and wrote test.
>>>>>>>>
>>>>>>>> open webrev at 
>>>>>>>> http://cr.openjdk.java.net/~coleenp/6909265.01/webrev
>>>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-6909265
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Coleen
>>>>>>
>>>>
>>