RFR: 8253332: ZGC: Make heap views reservation platform independent [v3]

Per Lidén pliden at openjdk.java.net
Mon Sep 21 10:04:38 UTC 2020


On Mon, 21 Sep 2020 06:17:10 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>>> I choose to prefix the OS dependent functions with os_. For consistency, initialize_os should have been renamed as
>>> well, but the plan is to change that in a separate patch that splits that function into two, so I skipped it for now.
>> 
>> May I suggest that we instead just keep to the convention we have (here and in other classes) and use an _os suffix on
>> these new functions?
>
>> > I choose to prefix the OS dependent functions with os_. For consistency, initialize_os should have been renamed as
>> > well, but the plan is to change that in a separate patch that splits that function into two, so I skipped it for now.
>> 
>> May I suggest that we instead just keep to the convention we have (here and in other classes) and use an _os suffix on
>> these new functions?
> 
> There's no strong convention that we postfix with os. During the Windows port I introduced two usages, both named
> initialize_os. Now when more OS specific code is added, and I need to split initialize_os into two function, the
> postfix makes it less clear what the OS-specific API extension points are.  Some of my motivations for going with a
> prefix instead: 1) HotSpot already does that in other places. See the pd_ functions. For example, pd_commit_memory vs
> commit_memory.
> 2) Splitting initialize_os wouldn't create an infix:
> os_initialize_before_reserve
> os_initialize_after_reserve
> vs
> initialize_os_before_reserve
> initialize_os_after_reserve
> 
> 3) Easily recognized group of os specific funtions
> os_initialize_before_reserve
> os_initialize_after_reserve
> os_reserve
> os_unreserve
> 
> 4) reserve_os sounded worse than os_reserve (to me)
> 
> I was considering splitting it out into a sub-interface, just to not have to deal with the *fix discussion/problem, but
> decided against it since the implementations needed member variables from ZVirtualMemoryManager.

@stefank That convention actually goes back to when ZGC was first integrated, with *_platform. However some of
those *_platform functions has been removed over time as they were no longer needed. When the Windows port came in we
said we should move away from *_platform to *_os, *_cpu and *_os_cpu to be more explicit and cater for potential future
needs. To me, the suffix-style reads a bit better, the relationship between e.g. initialize() and initialize_os()
becomes a bit more obvious, and it maps well to the platform specific file name convention we have in HotSpot
(e.g. *_linux_x86.hpp). Having said that, I'm ok with changing the convention to prefix-style.

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

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



More information about the hotspot-gc-dev mailing list