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 14:30:35 UTC 2018
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