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

[David Holmes]

This conversation is fragmenting across the two mailing lists - probably 
best to just discuss on the larger one: hotspot-dev.

I missed this response before sending my other responses, so apologies 
for any confusion as I refine my position.

On 11/10/2012 7:12 AM, Nils Loodin wrote:
> On Oct 10, 2012, at 22:29 , Vitaly Davidovich wrote:
>>
>> In allocation.cpp, "dothrow" constant is assigned std::nothrow_t but
>> isn't nothrow_t for *not* throwing? This constant is then used to
>> actually signal oom in one of the methods - seems odd
>>
>>
> std::nothrow is for not throwing, and is of the type std::nothrow_t.
>
> It is a bit of trickery, suggested by Keith McGuigan, which made the
> code I did much simpler.
> which we already had a nothrow_t overload. Having this other nothrow_t,
> which instead means DO throw, we can have this with a default argument
> on the methods
> that could be implemented with both kinds and then have a simple switch
> to see which when it was called with. either it was called with
> std::nothrow, in which case we do not throw on oom, or it was called
> with dothrow, in which case we does.

But we never "throw" anything so having a doThrow variant is even more 
inappropriate than the existing noThrow variant. At least with the 
existing nothrow, we were simply adopting the standard C++ operator new 
definitions to get an operator new that returned null. (If the constant 
had been called std::return_null rather than std::nothrow it would have 
matched our non-exception-based usage of "new" much more cleanly!)

>> Also, in ResourceObject::operator new() there's an assertion that
>> nothrow_t == std::nothrow - what's the purpose? Why would someone use
>> this overload if they're not looking to skip bad_alloc? What sort of
>> error are you thinking of here?
>>
>>
> These are the overloaded operator new():s which takes a std::nothrow_t.
> Since we defined a dothrow of the same type, it would be bad if someone
> accidentally called these operator news with dothrow, instead of
> std::nothrow.
> For instance, someone might do foo = new (dothrow) MyClass() in error.

That's an argument against not having this form of "new" in the first place.

>> Finally, I think the idiomatic way to do this type of stuff is to have
>> a separate overload of the alloc function that accepts nothrow_t
>> whereas you seem to have functions that take nothrow_t as a parameter,
>> but then you check if its the "dothrow"
>>
> It is sort of loosely following the pattern in Thread::operator new,
> which takes a std::nothrow_t and then calls other functions with a boolean.
> Me and Keith argued a bit and came to the conclusion that these function
> calls (" allocate(mySize, false)") would look much clearer in the
> callsite if it read "allocate(mySize, std::nothrow)" instead. otherwise,
> you'd have to dig around to see what was going on.

Okay, so I'm getting a clearer picture here now. There are two levels of 
allocation functions:
- operator new
- other stuff used by operator new

I think std::nothrow should only ever be seen in conjunction with 
operator new - where it belongs. If new then uses other allocation 
functions then they should use a different mechanism - like the Enum - 
to indicate how to handle allocation failures.

If the above "rules" are already violated then we should change the 
existing code.

David
-----

>
>> constant inside to see if oom should be reported. It seems cleaner to
>> have overloads where the one taking nothrow_t automatically means
>> don't throw?
>>
>>
> That kind of implementation actually lead to a lot of boilerplate code
> where the two functions overloaded would each call a third common method
> with a specific boolean.
>
> Hope this clears up stuff!
>
> Regards,
> Nils Loodin
>>
>> Thanks
>>
>> Sent from my phone
>>
>> On Oct 10, 2012 4:02 PM, "Nils Loodin" <nils.loodin at oracle.com
>> <mailto:nils.loodin at oracle.com>> wrote:
>>
>>     Hi all!
>>
>>     When looking to implement a feature where I wanted to do some
>>     allocations without the vm calling vm_exit_no_memory() when not
>>     able to allocate, I discovered this wasn't possible with
>>     ResourceObj-type obects. (It was however implemented in
>>     CHeapObj-type objects.
>>
>>     Here's a patch to implement that.
>>
>>     Now we can use (for instance) jcmd on a running production vm
>>     without fear that we will bring the vm down if the jcmd commands
>>     does allocations.
>>     (The commands themselves have to be implemented with this in mind
>>     though.)
>>
>>     Webrev here:
>>     http://cr.openjdk.java.net/~nloodin/8000617/webrev.01/
>>
>>     Regards,
>>     Nils Loodin
>>
>
>