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

Ioi Lam ioi.lam at oracle.com
Tue Apr 16 11:44:39 PDT 2013 

If you plan to fix 8012272 soon, I think it's better to leave the 
HandleMark code as-is in thread.cpp. There is no reason to keep churning 
this code and add stuff like delete operators, which many people, 
including myself, don't really understand.

   // the handle mark links itself to last_handle_mark
   new HandleMark(this);

...

   delete last_handle_mark();

- Ioi

On 04/16/2013 09:06 AM, Yumin Qi wrote:
> Hi, all
>
>   After several rounds of review, now there is one question of first 
> (and last)HandleMark creation/deleting in Thread constructor, how to 
> delete the last handle mark in Thread destructor.  With the new bug 
> filed (8012272), this code may be removed, so the issue will be gone. 
> In this fix, will keep handle mark creation/deleting here in 
> ctor/dtor.  For constructing handle mark, seems no arguing about that, 
> now it is the deleting.
>
>   Which way to delete the object? I need your opinion.
>
> Thanks
> Yumin
>
>
>
>
>
>
> On 4/16/2013 5:56 AM, David Holmes wrote:
>> On 16/04/2013 10:39 PM, Zhengyu Gu wrote:
>>> On 4/15/2013 7:20 PM, David Holmes wrote:
>>>> 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.
>>>>
>>> Yes, I meant placement delete operator, which takes two pointers.
>>
>> But you can only call that explicitly as a function. The delete 
>> expression will never use it, so I don't see how it applies. We need 
>> to call the destructor without de-allocating and the only way I can 
>> see to do that is to call the destructor explicitly.
>>
>> David
>> -----
>>
>>
>>> -Zhengyu
>>>
>>>
>>>> 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
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>

More information about the hotspot-runtime-dev mailing list