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

[David Holmes]

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

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