RFR: 8265066: Split ReservedSpace constructor to avoid default parameter

Thomas Schatzl tschatzl at openjdk.java.net
Thu Apr 15 08:57:34 UTC 2021


On Tue, 13 Apr 2021 09:34:31 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

> Please review this change to change this constructor in ReservedSpace:
> 
>   // Initialize the reserved space with the given size. If preferred_page_size
>   // is set, use this as minimum page size/alignment. This may waste some space
>   // if the given size is not aligned to that value, as the reservation will be
>   // aligned up to the final alignment in this case.
>   ReservedSpace(size_t size, size_t preferred_page_size = 0);
> 
> 
> I propose to split this constructor into two instead of having the default parameter. This makes the implementation more straight forward. I also make the single argument constructor explicit to avoid implicit conversion. There was one place where we relied on implicit conversion and this has been updated to explicitly create the ReservedSpace.
> 
> This cleanup is another step towards:
> [JDK-8261527: Record page size used for underlying mapping in ReservedSpace](https://bugs.openjdk.java.net/browse/JDK-8261527)

src/hotspot/share/memory/virtualspace.cpp line 58:

> 56: 
> 57: ReservedSpace::ReservedSpace(size_t size, size_t preferred_page_size) : _fd_for_heap(-1) {
> 58:   assert(is_power_of_2(preferred_page_size), "invariant");

Shouldn't this be something like `is_one_of_available_page_sizes` (or potentially 0 if this means "any")? This assert hasn't been in the original code, and it does not seem correct, so maybe it's not worth putting here too?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3457


More information about the hotspot-dev mailing list