RFR(XS): JDK-8214272: Don't use memset to initialize arrays of MemoryUsage in memoryManager.cpp

Dmitry Chuyko dmitry.chuyko at bell-sw.com
Tue Nov 27 15:19:13 UTC 2018


The hint is from Kim Barrett.

Surprisingly exactly this kind of warning is not silent just in 2 
places. I added matcher.cpp and re-submitted dev-submit tests.

webrev: http://cr.openjdk.java.net/~dchuyko/8214272/webrev.01/

-Dmitry

On 11/27/18 5:39 PM, Roman Kennke wrote:
> Oh. Right. There is always something to learn about C++ :-) Your patch
> looks good then. And actually, may want to fix 'my' change too, if you
> feel like it. And I believe gcc8 prints more similar warnings.
>
> Thanks,
> Roman
>
>
>> Doesn't "= MemoryUsage()" actually imply "delete onstack MemoryUsage"
>> after copying? Probably safe here but it may be not safe in general.
>>
>> -Dmitry
>>
>> On 11/27/18 5:14 PM, Roman Kennke wrote:
>>> I think Aleksey's version is clearer. I used a similar approach in a fix
>>> for the same issue:
>>>
>>> http://hg.openjdk.java.net/jdk/jdk/rev/710e5a66a64e
>>>
>>> Roman
>>>
>>>
>>>> Placement new sets values placed in arrays. The arrays as a whole are
>>>> managed as before. I.e.:
>>>>
>>>> GCStatInfo::~GCStatInfo() {
>>>>     FREE_C_HEAP_ARRAY(MemoryUsage*, _before_gc_usage_array);
>>>>     FREE_C_HEAP_ARRAY(MemoryUsage*, _after_gc_usage_array);
>>>> }
>>>>
>>>> -Dmitry
>>>>
>>>> On 11/27/18 5:00 PM, Aleksey Shipilev wrote:
>>>>> On 11/27/18 2:51 PM, Dmitry Chuyko wrote:
>>>>>> rfe: https://bugs.openjdk.java.net/browse/JDK-8214272
>>>>>> webrev: http://cr.openjdk.java.net/~dchuyko/8214272/webrev.00/
>>>>> Ummmm. Aren't you leaking memory by calling "new" without "delete"?
>>>>>
>>>>> Consider this instead:
>>>>>
>>>>> diff -r a554db76b2e9 src/hotspot/share/services/memoryManager.cpp
>>>>> --- a/src/hotspot/share/services/memoryManager.cpp      Fri Nov 23
>>>>> 11:22:31 2018 +0100
>>>>> +++ b/src/hotspot/share/services/memoryManager.cpp      Tue Nov 27
>>>>> 14:56:09 2018 +0100
>>>>> @@ -169,7 +169,8 @@
>>>>>       _start_time = 0L;
>>>>>       _end_time = 0L;
>>>>> -  size_t len = _usage_array_size * sizeof(MemoryUsage);
>>>>> -  memset(_before_gc_usage_array, 0, len);
>>>>> -  memset(_after_gc_usage_array, 0, len);
>>>>> +  for (int i = 0; i < _usage_array_size; i++) {
>>>>> +    _before_gc_usage_array[i] = MemoryUsage();
>>>>> +    _after_gc_usage_array[i]  = MemoryUsage();
>>>>> +  }
>>>>>     }
>>>>>
>>>>> See, for example:
>>>>>     https://bugs.openjdk.java.net/browse/JDK-8213745
>>>>>
>>>>> -Aleksey
>>>>>



More information about the hotspot-gc-dev mailing list