RFR: 8010992: Remove calls to global ::operator new[] and new
David Holmes
david.holmes at oracle.com
Thu Apr 18 02:52:05 UTC 2013
Hi Yumin,
Only thing missing - as discussed - is that HandleMark should now extend
StackObj, with a comment about the special CHeap HandleMark in thread.
Thanks for your perseverance on this.
David
On 18/04/2013 3:54 AM, Yumin Qi wrote:
> 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