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

[Yumin Qi]

Thanks, David

On 4/12/2013 1:46 AM, David Holmes wrote:
> Hi Yumin,
>
> This looks pretty good.
>
> On 12/04/2013 4:12 PM, Yumin Qi wrote:
>>    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/>
>
> src/share/vm/classfile/altHashing.cpp
>
> Can these be directly stack-allocated arrays instead of C-Heap ?
>
Yes
> ---
>
> allocation.cpp
>
> 672 // %% note this is causing a problem on solaris debug build. the 
> global
> 673 // new is being called from jdk source and causing data corruption.
> 674 // 
> src/share/native/sun/awt/font/fontmanager/textcache/hsMemory.cpp::hsSoftNew
>
> Is this still an issue?
>
> As a general comment do we need all the operator new[] additions?
>
No longer an issue, the code mentioned in jdk has been removed.
> ---
>
> allocation.inline.hpp
>
> Can we reduce code duplication by changing, eg:
>
> #ifdef ASSERT
>     void* p = (void*)AllocateHeap(size, F, (caller_pc != 0 ? caller_pc 
> : CALLER_PC));
>     if (PrintMallocFree) trace_heap_malloc(size, "CHeapObj-new", p);
>     return p;
> #else
>     return (void *) AllocateHeap(size, F, (caller_pc != 0 ? caller_pc 
> : CALLER_PC));
> #endif
>
> to
>
>     void* p = (void*)AllocateHeap(size, F, (caller_pc != 0 ? caller_pc 
> : CALLER_PC));
> #ifdef ASSERT
>     if (PrintMallocFree) trace_heap_malloc(size, "CHeapObj-new", p);
> #endif
>     return p;
>
Yes, that is better.
> ---
>
> src/share/vm/memory/cardTableModRefBS.cpp
>
> 86   for (i = 0; i < max_covered_regions; i++) {
> 87     _covered[i].set_start(NULL)  ; _covered[i].set_word_size(0)  ;
> 88     _committed[i].set_start(NULL); _committed[i].set_word_size(0);
> 89   }
>
> this appears to be a semantic change, which leads me to ask:
>
> Given "new MemRegion[1]" vs NEW_C_HEAP_ARRAY(...,1,...) what are the 
> array contents initialized to in each case?
>
Ii should be changed to use NEW_C_HEAP_OBJECT, but MemRegion is a 
_ValueObj which should not be new'ed.   The loop here is just doing that 
what the default ctor do.
> ---
>
> src/share/vm/memory/cardTableRS.cpp
>
>   68 CardTableRS::~CardTableRS() {
>   69   if (_last_cur_val_in_gen) {
>   70     // FREE_C_HEAP_ARRAY(jbyte, _last_cur_val_in_gen, mtInternal);
>   71   }
>   72 }
>
> Was this an existing memory leak? I'm noticing that most uses of "new 
> X[n]" don't have a corresponding delete!
>
> But why is this commented out?
That is a mistake, I will uncomment it back.
>
> ---
>
> src/share/vm/runtime/handles.hpp
>
> initialize_thread shouldn't need to be public. What is calling it?
>
> ---
>
> src/share/vm/runtime/thread.cpp
>
> These changes don't look right - why doesn't HandleMark extend one of 
> the allocation base classes ?
>
The HandleMark should be an StackObj, but we need to create it in heap 
in thread creation. This part is confusing me too.  This is the only 
place to create it in heap, all the usage is a StackObj.

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