review request (S) - 8014611: reserve_and_align() assumptions

[Thomas Schatzl]

Hi,

On Tue, 2013-05-21 at 14:27 -0700, John Coomes wrote:
> Bengt Rutisson (bengt.rutisson at oracle.com) wrote:
> > On 5/21/13 2:46 AM, John Coomes wrote:
> 
> > In both ReservedSpace::save_raw_base_and_size() and 
> > ReservedSpace::failed_to_reserve_as_requested() you have the return type 
> > on a separate line. That does not seem to be the style in the rest of 
> > the file.
> 
> Ok; I changed the line breaks.  All the above changes are in a new
> webrev (at the same location):
> 
> http://cr.openjdk.java.net/~jcoomes/8014611-reserve-and-align/
> 
> Thanks again for the review.  I'll push this unless there are
> objections.

One minor note: I think not setting _raw_base and _raw_size inn
save_raw_base_and_size() in all cases can lead to junk in these
variables which might be confusing when debugging.

So the check for _raw_base == NULL in ReservedSpace::release_memory() is
misleading/ineffective, as now, even if os::can_release_partial_memory()
is true, it likely contains junk that is non-zero.

The check for os::can_release_partial_memory() in
ReservedSpace::release_memory() helps guaranteeing that the wrong method
is not called though.

(If I follow the code correctly).

I.e. the set_raw_base_and_size(NULL, 0) are ineffective when
os::can_release_partial_memory() is true.

I'd prefer it if _raw_base and _raw_size really contained NULL/0 in case
of os::can_release_partial_memory() is true, and make the (single) call
to save_raw_baw_and_size() in ReservedSpace::reserve_and_align()
conditional on os::can_release_partial_memory().

The change further does not update copyright dates :)

Thanks,
Thomas