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

[David Holmes]

On 16/04/2013 3:44 PM, Yumin Qi wrote:
> For HandleMark,
>
>
> 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.
>>
>
> The default constructor for HandleMark:
> inline HandleMark::HandleMark() {
>    initialize(Thread::current());
> }
>
> In thread.cpp, Thread::Thread(), the last HandleMark is created in heap,
> -  new HandleMark(this);
> -
> +  HandleMark *hm = NEW_C_HEAP_OBJ(HandleMark, mtThread);
> +  hm->initialize(this);
> The original code, is calling the constructor with 'this'.  Unless we
> code a macro to pass 'this' as argument, which looks specific not
> generic, to call the constructor.  When call default constructor here,
> will assert current thread is NULL --- the real native thread has not
> created yet , the thread id is not cached so we get back a NULL for
> Thread::current(). That is, we could not call the default constructor at
> this time, this change seems strange but works --- first created the
> object without calling constructor in heap, then initialize with 'this'.

I understand why you had to make the change to make the code work, but 
it is still a horrible change to have to make. :)

> The problem is ~HandleMark is not called this way. Need to have a
> version to call dtor which seems ugly.
>
> The handle mark here for what? I could  not spot any handle allocated
> from this point in Thread creation.

As I commented in the new CR I also can not see why we need this initial 
HandleMark. It's presence greatly complicates things.

David

> Thanks
> Yumin
>
>
>> ---
>>
>> 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
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>