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

[harold seigel]

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