RFR (S): 8238999: Remove MemRegion custom new/delete operator overloads

[Jiangli Zhou]

On Fri, Feb 14, 2020 at 3:05 PM Kim Barrett <kim.barrett at oracle.com> wrote:
>
> > On Feb 14, 2020, at 10:05 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> >
> > Hi all,
> >
> >  can I have reviews for this small change to the MemRegion class to remove unnecessary new/delete overloads from MemRegion.
> >
> > They return NULL if there is not enough memory. This is uncommon to do in Hotspot code.
> >
> > All uses in the code either checks whether the allocation is non-NULL and then terminates the VM, or will just crash too.
> >
> > It is easier to just replace the new[] calls with NEW_C_HEAP_ARRAY allocations and do the initialization manually.
> >
> > cc'ing runtime because Coleen added the new operator for working around a Metaspace issue in JDK-8021954 years ago.
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8238999
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8238999/webrev/
> > Testing:
> > hs-tier1-4
> >
> > Thanks,
> >  Thomas
>
> ------------------------------------------------------------------------------
> src/hotspot/share/memory/memRegion.hpp
>   96   // Creates and initializes an array of MemRegions of the given length.
>   97   static MemRegion* create(uint length, MEMFLAGS flags);
>
> A function named "create" suggests to me creating a single object, not
> an array.  Perhaps "make_array" or "create_array" or "new_array"?

+1. I had the same thoughts when looking at the webrev.1.

Best regards,
Jiangli

>
> ------------------------------------------------------------------------------
>
> Other than that, looks good.  I don't need a new webrev for using any
> of the suggested names.
>
> I noticed the memory leak in map_heap_data, but see that you filed a
> separate bug for that, and already have a reviewed fix for it.
>