RFR: 8000617: It should be possible to allocate memory without the VM dying.

[Nils Loodin]

So in general we have two competing wishes here:

1. Use std::nothrow, in operator new() and enum in the allocation methods. (basically webrev 03)
Pros: more c++ standard-ish
cons: more boiler plate, not descriptive of what actually happens, two different mechanisms.


2. Use enums everywhere (basically a bugfree version of 04):
pros: more accurate description, cleaner implementation
cons: it's an instance of "not invented here"-syndrom, ie, we're making up our own stuff.

So.. discuss! John, what do you think having seen the different implementations? It would be nice to be able to get in any version of it so I can develop the feature that will depend on it... :)

Regards,
Nisse

On Oct 12, 2012, at 17:03 , Coleen Phillimore <coleen.phillimore at oracle.com> wrote:

> 
> I still prefer webrev 03 to webrev 04.   std::nothrow as a parameter means something to any C++ developer and AllocationStrategy::OOM only means something to a hotspot developer.  I think we should use the standard C++ apis when possible.  The duplication of operator new is not significant.   Plus if someone adds new (std::nothrow) call or you forgot to clean up one, it think it will overload to the std::operator new() which could lead to a subtle bug.
> 
> Coleen
> 
> On 10/12/2012 10:40 AM, Nils Loodin wrote:
>> 
>> 
>> 
>> 12 okt 2012 kl. 16:25 skrev Keith McGuigan<keith.mcguigan at oracle.com>:
>> 
>>> May as well lift the allocation fail strategy up into Thread::allocate() as well, replacing the boolean parameter there.
>>> 
>> Ah, good idea.
>> 
>>> I think you're replacements of new(std::nothrow) XXX() wit new (EXIT_OOM) XXX() are all reversed: if it was nothrow you want to replace it with RETURN_NULL.
>>> 
>>> -
>> Argh, darn, you're right of course.
>> 
>> Will fix.
>> 
>>> - Keith
>>> 
>>> On Oct 12, 2012, at 9:14 AM, Nils Loodin wrote:
>>> 
>>>> Hey guys!
>>>> 
>>>> Thanks for yet another round of informative code reviews!
>>>> So I got rid of all the instances of std::nothrow throughout the code and replaced them with a new shiny and descriptive enum.
>>>> 
>>>> Was able to fold together a few specialized operator new() because of it, with more shared code as a result.
>>>> 
>>>> What do you think now?
>>>> http://cr.openjdk.java.net/~nloodin/8000617/webrev.04/
>>>> 
>>>> Have a nice weekend!
>>>> 
>>>> Regards,
>>>> Nils Loodin
>>>> 
>>>> 
>>>> On 10/12/2012 06:47 AM, David Holmes wrote:
>>>>> On 12/10/2012 2:21 PM, David Holmes wrote:
>>>>>> Hi Nils,
>>>>>> 
>>>>>> There are a number of existing bugs/RFEs in this area - not sure we
>>>>>> needed yet another.
>>>>>> 
>>>>>> On 11/10/2012 10:55 PM, Nils Loodin wrote:
>>>>>>> Hey guys.
>>>>>>> 
>>>>>>> Your comments on this issue are great! So here's another version that
>>>>>>> uses an enum instead of std::nothrow_t trickery!
>>>>>>> 
>>>>>>> http://cr.openjdk.java.net/~nloodin/8000617/webrev.03/
>>>>>> I dislike the std::nothrow usage that NMT introduced (there aren't even
>>>>> Apologies it wasn't NMT that introduced this - it is a bit older than
>>>>> that: April 2011. See
>>>>> https://jbs.oracle.com/bugs/browse/JDK-7036747
>>>>> 
>>>>> and some comments in
>>>>> 
>>>>> https://jbs.oracle.com/bugs/browse/JDK-4719004
>>>>> 
>>>>> Till then we had avoided any use of C++ std:: stuff - particularly
>>>>> anything related to the exception mechanism, which we don't use at all.
>>>>> 
>>>>>> any exceptions involved!) but introducing a completely different
>>>>>> mechanism seems counter-productive. I prefer the "alloc fail strategy"
>>>>>> approach but would like to see this solved holistically, replacing
>>>>>> std::nothrow. Given there exist bigger RFE's to have the VM handle all
>>>>>> OOM situations gracefully rather than just aborting, I'd rather not see
>>>>>> just another point-patch put in place.
>>>>> The bigger problem is probably still too big to really handle - so
>>>>> either copy what we already introduced for CHeapObj to ResourceObj for
>>>>> consistency, or replace both with something better.
>>>>> 
>>>>> Cheers,
>>>>> David
>>>>> -----
>>>>> 
>>>>>> For the record I found the original proposal extremely confusing:
>>>>>> nothrow_constant vs throw_constant.
>>>>>> 
>>>>>> Sorry.
>>>>>> 
>>>>>> David
>>>>>> -----
>>>>>> 
>>>>>>> Regards,
>>>>>>> Nils Loodin
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> On 10/11/2012 01:21 PM, Dmitry Samersoff wrote:
>>>>>>>> Keith,
>>>>>>>> 
>>>>>>>> Personally, I would prefer explicit AllocWithoutThrow(...) function.
>>>>>>>> 
>>>>>>>> -Dmitry
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On 2012-10-11 03:40, Keith McGuigan wrote:
>>>>>>>>> Personally, I don't have strong feelings on how this is implemented,
>>>>>>>>> other than it should be done in a way that's maintainable going
>>>>>>>>> forward
>>>>>>>>> and easily understandable by future generations of hotspot developers.
>>>>>>>>> With this in mind, the only potential solution that I don't like is
>>>>>>>>> using a boolean with naked true/false values as discriminators.
>>>>>>>>> 
>>>>>>>>> Using some sort of "failure mode" parameter is the natural way to do
>>>>>>>>> this, whether it be enums, std::nothrow_t, or whatever. Since
>>>>>>>>> std::nothrow_t already has a type and one value, and is already
>>>>>>>>> present
>>>>>>>>> in the few places we're interested in, it seemed easy to simple just
>>>>>>>>> add
>>>>>>>>> a new value to use. However if this ends up being confusing because
>>>>>>>>> this is not the normal use of std::notype_t, then fine, we can do
>>>>>>>>> something else.
>>>>>>>>> 
>>>>>>>>> --
>>>>>>>>> - Keith