RFR: JDK-8312018: Improve reservation of class space and CDS [v4]

[Thomas Stuefe]

On Wed, 16 Aug 2023 06:19:13 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/runtime/os.cpp line 1813:
>> 
>>> 1811: // Given an address range [min, max), attempts to reserve memory within this area, with the given alignmend.
>>> 1812: // If randomize is true, the location will be randomized.
>>> 1813: char* os::attempt_reserve_memory_between(char* min, char* max, size_t bytes, size_t alignment, bool randomize) {
>> 
>> Could you avoid the bool parameter and use the flag RandomizeClassSpaceLocation directly? This means you would have to set that flag in tests, but that seems better than having boolean args there.
>> Also, it looks to me like the block that calculates the attach points (either randomized or not) Could be broken out into helper functions (one for randomized, one for non-randomized and one that dispatches between the two)?
>
> `os::attempt_reserve_memory_between` is a generic API, usable for other things too. One obvious use case would be allocating the heap (replacing all that manual lets-reserve-the-heap-in-lower-regions in virtualSpace.cpp); another one Shenandoah CollectionSet; and some more. I don't want to impose the "randomize" decision on all callers.

> Could be broken out into helper functions (one for randomized, one for non-randomized and one that dispatches between the two)?

See my answer to @iklam https://github.com/openjdk/jdk/pull/15041/files#r1290988328 . I spent considerable time trying to come up with a good split along the lines of randomized/non-randomized. All variants I came up with would lead to code duplication that I'm sure would be criticised by other reviewers. Let's see what others think.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15041#discussion_r1295429999