RFR: JDK-8256287: [windows] add loop fuse to map_or_reserve_memory_aligned [v2]

[Ludovic Henry]

On Mon, 16 Nov 2020 12:24:07 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Hi,
>> 
>> may I please have reviews for this little fix:
>> 
>> On windows, `map_or_reserve_memory_aligned()` attempts to allocate aligned memory by:
>> 1) reserving larger area
>> 2) releasing it
>> 3) attempting to re-reserve into the vacated address space a smaller area at the aligned start address
>> 
>> Since this may fail (between (2) and (3) someone may have grabbed part of that address space concurrently), we do this in a loop. However, when failing to release it we will loop-reserve endlessly.
>> 
>> This is one of the reasons for JDK-8255954 whose root cause will be fixed with JDK-8255978. But we should guard against endless loops independently from that.
>> 
>> This patch:
>> - limits the number we try this to 20. If in 20 cases someone else grabs address space concurrently, something is off...
>> - now correctly handles the return code of the release operation (1): in debug we assert, since if release_memory() fails something is wrong and we should take a look. In the release case, we now return an error.
>> 
>> 
>> Thanks!
>
> Thomas Stuefe has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Remove blank before unary ++
>  - feedback alexey

I'm not a reviewer but it looks good to me.

src/hotspot/os/windows/os_windows.cpp line 3152:

> 3150: 
> 3151:   char* aligned_base = NULL;
> 3152:   static const int max_attempts = 20;

Stylistic review as well: I'd put that counter in a loop like `for (int attempts = 0; aligned_base == NULL && attempts < 20 /* max attemps */; attemps++) { ... }`. That would clearly indicate that it is a counted loop.

Otherwise, now that you can return `NULL` from this function, have you checked that all uses of this function deal with a NULL-returned value correctly? From a cursory glance, it looks good to me.

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

Marked as reviewed by luhenry (Author).

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