RFR: 8290840: Refactor the "os" class [v3]
Ioi Lam
iklam at openjdk.org
Tue Aug 2 06:46:44 UTC 2022
On Mon, 1 Aug 2022 18:55:59 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>>
>> fixed typo
>
> src/hotspot/os/aix/os_aix.hpp line 180:
>
>> 178: static bool get_meminfo(meminfo_t* pmi);
>> 179:
>> 180: // PPC only
>
> Since your are here, remove comment? There has not been an AIX on something other than power for ~25 years.
Fixed
> src/hotspot/os/aix/os_aix.inline.hpp line 37:
>
>> 35: inline bool os::zero_page_read_protected() {
>> 36: return false;
>> 37: }
>
> I have the feeling many of these functions are really just global constants and that the coding could be simplified further in followup RFEs.
Yeah lets do more clean up in follow-up RFEs.
> src/hotspot/os/linux/os_linux.cpp line 4492:
>
>> 4490: // perform the workaround
>> 4491: Linux::capture_initial_stack(JavaThread::stack_size_at_create());
>> 4492: Linux::workaround_expand_exec_shield_cs_limit();
>
> You could move `workaround_expand_exec_shield_cs_limit()` into this file, declare it static, and remove all mentionings from headers. It now lives in os_linux_x86.cpp, but since it has to be wrapped with !zero !64bit already it is an odd misfit anyway. So its not much more "wrong" for it to live here.
Fixed.
> src/hotspot/share/runtime/os.hpp line 92:
>
>> 90: // The Porting APIs declared as "inline" in os.hpp must be
>> 91: // implemented in one of the two .inline.hpp files, depending on whether
>> 92: // the feature is generic to the OS, or specific to a particular CPU
>
> generic? Did you mean specific? E.g. "depending on whether the feature is specific to a certain OS or an OS+CPU combination" ?
I clarified the comment:
// The Porting APIs declared as "inline" in os.hpp must be
// implemented in one of the two .inline.hpp files, depending on
// whether the feature is specific to a particular CPU architecture
// for this OS.
> src/hotspot/share/runtime/os.inline.hpp line 37:
>
>> 35: // Provide default empty implementation.
>> 36:
>> 37: #ifndef PLATFORM_PRINT_NATIVE_STACK
>
> Could you rename this to HAVE_PLATFORM_PRINT_NATIVE_STACK?
Fixed
-------------
PR: https://git.openjdk.org/jdk/pull/9600
More information about the hotspot-dev
mailing list