RFR: 8266349: Pass down requested page size to reserve_memory_special

Thomas Stuefe stuefe at openjdk.java.net
Wed May 5 12:36:51 UTC 2021


On Wed, 5 May 2021 10:29:32 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

> Please review this enhancement that passes the requested page size in `ReservedSpace` down to the OS-layer.
> 
> **Summary**
> After recent refactoring `ReservedSpace` now store the page size used for a reservation. To make this more consistent this change passes down the page size from the `ReservedSpace` down to the OS-layer when using explicit large pages. Currently this is not really needed because we only support one large page size during the lifetime of the JVM, but this will change with PR #1153. That PR will be simpler to implement if the requested page size is passed down from the layers above. 
> 
> **Testing**
> Local testing together with parts of PR #1153. Mach5 tier 1-3

Very good. 

This is not only cosmetic but functional, no? Since before we would always os::large_page_size(); now, in the case of ReservedSpace::ReservedSpace(size), we use whatever os::page_size_for_region wants us to use. Lets see if any errors creep up.

..Thomas

src/hotspot/os/linux/os_linux.cpp line 3968:

> 3966:   assert(is_aligned(alignment, os::vm_allocation_granularity()), "Must be");
> 3967:   assert(is_power_of_2(page_size), "Must be");
> 3968:   assert(bytes >= page_size, "Shouldn't allocate large pages for small sizes");

We could replace these two asserts with _page_sizes.contains.

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

> 170: 
> 171: static char* reserve_memory_special(char* requested_address, const size_t size,
> 172:                                     const size_t alignment, const size_t page_size, bool exec) {

I don't think the const makes sense... nor for the other arguments (but when in Rome...)

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

Marked as reviewed by stuefe (Reviewer).

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


More information about the hotspot-dev mailing list