RFR: 8010992: Remove calls to global ::operator new[] and new
Yumin Qi
yumin.qi at oracle.com
Tue Apr 16 20:52:25 UTC 2013
Hi, all
new webrev at
http://cr.openjdk.java.net/~minqi/8010992/webrev3
Changes: NEW_C_HEAP_ARRAY to create HandleMark in Thread::Thread,
use FREE_C_HEAP_OBJECT to free it in ~Thread.
quickSort.cpp, remove conditional includes.
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-gc-dev
mailing list