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

[Thomas Stuefe]

On Fri, 11 Aug 2023 07:23:27 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/runtime/os.cpp line 1790:
>> 
>>> 1788: // If randomize is true, the location will be randomized.
>>> 1789: char* os::attempt_reserve_memory_between(char* min, char* max, size_t bytes, size_t alignment, bool randomize) {
>>> 1790: 
>> 
>> I would suggest breaking this function down into smaller functions for readability.
>
> I wrecked my brain about how to split this function up, without coming to a solution I'm almost sure will be criticised for code duplication or obscurity.
> 
> The function sets up several common things before splitting into either randomized or non-randomized logic. If I split along randomized/non-randomized, whichever way I split, I would have either to duplicate most of the setup coding or give these daughter functions a very broad interface, which would make them not comprehensible. In my view, splitting makes sense only if you can describe the split-off parts in isolation well.
> 
> Also, any internal daughter functions resulting from the split would end up in the externally visible os namespace since they need to access the private `os::pd_attempt_reserve_..`, which I don't like.
> 
> I could attempt to split out the reservation alone (the loop calling os::pd_attempt_reserve...), e.g. to reserve from a given precomputed set of addresses. But that would require me to rewrite the non-randomized part that does not precompute those addresses, which I dislike. Again, this split-out variant would also end up in the externally visible os:: namespace.
> 
> I could split out part of the shuffling logic, and maybe I try that. But I feel not much brevity is gained from it, these sections are rather short.
> 
> Do you have any particular split in mind?

Okay, I factored out some of the smaller stuff into helper functions.

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

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