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

[Nils Loodin]

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
>