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

Yumin Qi yumin.qi at oracle.com
Mon Apr 15 17:01:39 UTC 2013


David,

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.
>
I agree, will file a separate CR for this problem to be fixed.
> ---
>
> 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 ?
>
In fact, CardTableRS and CardTableModRefBS are only created once when GC 
flavor specified. It will be cleaned at JVM exit, this is why there is 
no memory leak. To keep a better coding practice, I will add 
FREE_C_HEAP_OBJECT_ARRAY to call dtos, also to free other allocated 
arrays during ctos called.

> ---
>
> 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.
>
This is due to SunStudio compilation setting. It will issue error on 
solaris like:

src/share/vm/utilities/quickSort.cpp", line 192: Error: The function 
"AllocateHeap" must have a prototype.

in precompiled.hpp:

// Precompiled headers are turned off for Sun Studion,
// or if the user passes USE_PRECOMPILED_HEADER=0 to the makefiles.

#ifndef DONT_USE_PRECOMPILED_HEADER

I could add the including for all platforms, since it will be excluded 
by other platforms except for solaris --- precompiled.hpp already 
includes those two files. Adding #ifdef SOLARIS here to indicate they 
are only needed for Solaris.

I will send new webrev out.

Thanks
Yumin


> 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