RFR: 8303852: current_stack_region() gets called twice unnecessarily [v3]

[Patricio Chilano Mateo]

On Mon, 21 Aug 2023 03:46:10 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> We combine:
>> 
>> address os::current_stack_base();
>> size_t os::current_stack_size();
>> 
>> into
>> 
>> void os::current_stack_base_and_size(address* stack_base, size_t* stack_size)
>> 
>> and so avoid making two underlying system calls. The os/platform specific specializations are handled by the `current_stack_region` calls. It made sense to modify that to export the `base` directly rather than the `bottom`. In doing that change it made sense to standardise on the the style of the code used (variable names etc) to make it easier to see where the 5 different versions were the same and where they differ. Having 5 versions is unfortunate ( 3 BSD: one per arch - zero, x64, aarch64; and 2 linux: zero and everything else), but trying to combine them with ifdefs would be even worse in my opinion.
>> 
>> In theory there should be zero functional changes here.
>> 
>> Testing:
>>  - Tiers 1-3
>>  - All our internal builds in tiers 1-5
>>  - GHA
>> 
>> Thanks.
>
> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Promote os::<os>::current_stack_region to os::current_stack_base_and_size

Looks good to me.

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

> 5370: // ** P1 (aka bottom) and size are the address and stack size
> 5371: //    returned from pthread_attr_getstack().
> 5372: // ** P2 (aka stack top or base) = P1 - size)

Preexisting, but isn't P2 = P1 + size? Also an extra parenthesis was left at the end.

src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp line 714:

> 712: // ** P1 (aka bottom) and size are the address and stack size
> 713: //    returned from pthread_attr_getstack().
> 714: // ** P2 (aka stack top or base) = P1 - size)

Same thing here.

src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp line 192:

> 190: 
> 191:   if (rslt != 0)
> 192:     fatal("pthread_stackseg_np failed with error = %s", rslt);

%d

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

Marked as reviewed by pchilanomate (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15321#pullrequestreview-1592267815
PR Review Comment: https://git.openjdk.org/jdk/pull/15321#discussion_r1303448267
PR Review Comment: https://git.openjdk.org/jdk/pull/15321#discussion_r1303450521
PR Review Comment: https://git.openjdk.org/jdk/pull/15321#discussion_r1303452755