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

[coleen.phillimore at oracle.com]

Thanks Harold!
Coleen

On 2/2/18 9:00 AM, harold seigel wrote:
> Hi Coleen,
>
> This change looks good.
>
> Thanks, Harold
>
>
> On 2/2/2018 8:35 AM, coleen.phillimore at oracle.com wrote:
>>
>>
>> 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
>>>>>>>>
>>>>>>
>>>>
>>
>