RFR: 8010992: Remove calls to global ::operator new[] and new
Yumin Qi
yumin.qi at oracle.com
Wed Apr 17 17:54:11 UTC 2013
Hi, This version Change back the HandleMark creating in Thread creation
back to previous version and add more comments to allocation.hpp for
documenting the usage of newly introduced macros.
http://cr.openjdk.java.net/~minqi/8010992/webrev3
Thanks
Yumin
On 4/16/2013 1:52 PM, Yumin Qi wrote:
> 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