RFR: 8010992: Remove calls to global ::operator new[] and new

[David Holmes]

Adding back compiler and gc teams

On 16/04/2013 7:48 AM, Yumin Qi wrote:
> Zhenyu,
>
> On 4/15/2013 2:02 PM, Zhengyu Gu wrote:
>> Maybe you need a replacement delete operator, please check
>> http://www.cplusplus.com/reference/new/operator%20delete/
>>
> i can just do
>
> delete ((type *)&array_name[index]);
>
> so the destructor will be called, is this right?

No, it will also deallocate the memory unless you use a variant of 
delete (which is what I think Zhengyu was referring to) that doesn't 
actually deallocate the memory.

I think this is getting out of hand - if we go this path then we should 
simply have a custom operator new[] and operator delete[] and not need 
NEW_C_HEAP_ARRAY etc. But I don't think our usage is simple enough to 
actually do that. So if we need NEW_C_HEAP_OBJ_ARRAY that allocates and 
constructs then we need a FREE_C_HEAP_OBJ_ARRAY that destructs and then 
de-allocates.

I have no issue with calling destructors explicitly.

David
-----

> Thanks
> Yumin
>> Thanks,
>>
>> -Zhengyu
>>
>> On 4/15/2013 2:42 PM, Zhengyu Gu wrote:
>>>   623 #define FREE_C_HEAP_OBJECT_ARRAY(type, array_name, size, memflags)                  \
>>>   624   {                                                                                 \
>>>   625     if (array_name != NULL) {                                                       \
>>>   626       for (int index = 0; index < size; index ++) {                                 \
>>>   627         /* placement to call dtor on type */                                        \
>>>   628         ((type*)&array_name[index])->~type();                                       \
>>>   629       }                                                                             \
>>>   630       FREE_C_HEAP_ARRAY(type, array_name, memflags);                                \
>>>   631     }                                                                               \
>>>   632   }
>>>
>>>
>>> I really don't like this, calling dtor  ...
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>>
>>> On 4/15/2013 1:41 PM, Yumin Qi wrote:
>>>> new webrev at:
>>>>
>>>> http://cr.openjdk.java.net/~minqi/8010992/webrev2
>>>>
>>>> Can I have a review from GC team for this change?
>>>>
>>>> Thanks
>>>> Yumin
>>>>
>>>> On 4/14/2013 11:07 PM, David Holmes wrote:
>>>>> Hi Yumin,
>>>>>
>>>>> On 13/04/2013 7:07 AM, Yumin Qi wrote:
>>>>>> After take feedback and modify, new webrev
>>>>>>
>>>>>> http://cr.openjdk.java.net/~minqi/8010992/webrev1
>>>>>> <http://cr.openjdk.java.net/%7Eminqi/8010992/webrev1>
>>>>>
>>>>> I still find the HandleMark changes unsatisfactory. The CHeap
>>>>> allocated HandleMark in the Thread() constructor is a bit of a
>>>>> hack. HandleMarks should be stack-allocated. Plus we seem to leak
>>>>> that CHeap allocated HandleMark as we don't keep any pointer to it!
>>>>> I think this needs to be re-visited, but as a separate CR.
>>>>>
>>>>> ---
>>>>>
>>>>> allocation.hpp:
>>>>>
>>>>> If  NEW_C_HEAP_OBJECT_ARRAY invokes the ctor for each array element
>>>>> don't we need a new FREE_C_HEAP_OBJECT_ARRAY to invoke the dtor's
>>>>> before deallocating ?
>>>>>
>>>>> ---
>>>>>
>>>>> src/share/vm/utilities/quickSort.cpp
>>>>>
>>>>> Why does only Solaris need the allocation headers included ?? I
>>>>> would expect all platforms to need to the same headers here.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks
>>>>>> Yumin
>>>>>>
>>>>>> On 4/12/2013 10:45 AM, Zhengyu Gu wrote:
>>>>>>> .
>>>>>>>>> cardTableModRefBS.cpp
>>>>>>>>>   #87 and #88, why set_start(NULL) are needed?
>>>>>>>>>
>>>>>>>> This is default constructor does, here just copy that code.
>>>>>>>> Since we
>>>>>>>> did not call constructor by using this MACRO. It is a _ValueObj,
>>>>>>>> should not call new, but I think we can use NEW_C_HEAP_OBJ3, which
>>>>>>>> will call ctors.
>>>>>>> NEW_C_HEAP_OBJECT_ARRAY macro does invoke ctor ... that is what
>>>>>>> allocation.hpp #618 does.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> -Zhengyu
>>>>>>>
>>>>>>>
>>>>>>>>> carTableRS.cpp
>>>>>>>>>   #70, why it is commented out? If so, you don't need the dstor
>>>>>>>>>
>>>>>>>>>
>>>>>>>> See reply to David H.
>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Yumin
>>>>>>>>> -Zhengyu
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 4/12/2013 2:12 AM, Yumin Qi wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>>   Can I have your inputs for the fix?
>>>>>>>>>>   webrev:
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~minqi/8010992/webrev/
>>>>>>>>>> <http://cr.openjdk.java.net/%7Eminqi/8010992/webrev/>
>>>>>>>>>>
>>>>>>>>>>   Bug:  8010992: Remove calls to global ::operator new[] and new
>>>>>>>>>> https://jbs.oracle.com/bugs/browse/JDK-8010992
>>>>>>>>>>
>>>>>>>>>> Problem description:  Remove the usage of global operator ::new[]
>>>>>>>>>> and ::new. In hotspot debug build, disable the usage of global
>>>>>>>>>> new[] and new.  Hotspot does not throw c++ exceptions, but it
>>>>>>>>>> cannot prevent third party code to catch such exceptions.  By
>>>>>>>>>> disabling use of global operator new[] and new, we constrain the
>>>>>>>>>> exception disposal within hotspot. C++ classes (as same for
>>>>>>>>>> structs) in hotspot have to either extends from CHeapObj or
>>>>>>>>>> ResourceObj unless they are stack objects or values which have to
>>>>>>>>>> be from StackObj or _ValueObj respectively.  Or they have to
>>>>>>>>>> implement their own operator new[] or new.
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Yumin
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>>
>