RFR(xs): 8245035: Clean up os::split_reserved_memory()

[Thomas Stüfe]

Hi Coleen,

thanks for the review!

On Thu, May 14, 2020 at 5:57 PM <coleen.phillimore at oracle.com> wrote:

>
>
> http://cr.openjdk.java.net/~stuefe/webrevs/8245035--clean-up-os--split_reserved_memory()/webrev.00/webrev/src/hotspot/share/memory/virtualspace.hpp.udiff.html
>
>
> + // If split==false, the resulting space will be just a hotspt-internal
> representation
>
>
> typo: needs an 'o'
>
>
Fixed.


>
> http://cr.openjdk.java.net/~stuefe/webrevs/8245035--clean-up-os--split_reserved_memory()/webrev.00/webrev/src/hotspot/share/runtime/os.cpp.udiff.html
>
> I don't think we need a pd_ version to delegate to for this.  I guess
> it's a convention because the other memory allocation functions do it
> but maybe we can slowly end this bad convention.
>

Good point. I removed the pd_ implementation and now we just have to direct
implementations of os::split_reserved_memory().



> This looks like a very nice cleanup.
> Thank you!
> Coleen
>
>
I updated the webrev in place.

Thanks, Thomas


> On 5/14/20 11:45 AM, Thomas Stüfe wrote:
> > Hi,
> >
> > may I have reviews for this small cleanup please:
> >
> > jbs: https://bugs.openjdk.java.net/browse/JDK-8245035
> > webrev:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8245035--clean-up-os--split_reserved_memory()/webrev.00/webrev/
> >
> > I need this as a preparation for
> > https://bugs.openjdk.java.net/browse/JDK-8243535, which I'd like to
> review
> > separately.
> >
> > os::split_reserved_memory() should be cleaned up a bit. It is used to
> split
> > reserved memory regions in order to make them independent units. This
> only
> > matters when releasing them, so after splitting these regions should be
> > releasable independently from each other.
> >
> > This whole thing only matters on Windows, which is the only platform
> with a
> > non-empty implementation, since virtual memory can only be released as a
> > unit (opposed to mmap api, where sub regions can be unmapped
> individually).
> >
> > Improvements:
> > - the interface should be commented
> > - the "realloc" parameter can be removed. It has never not been true on
> all
> > code paths - had it been, it would have been an error on windows
> resulting
> > in loosing one of the two sides of the split.
> > - the many non-windows implementations can be combined in posix.cpp.
> >
> > Thank you,
> >
> > Thomas
>
>