RFR: JDK-8257588: Make os::_page_sizes a bitmask [v3]

Thomas Stuefe stuefe at openjdk.java.net
Fri Dec 4 06:05:11 UTC 2020


On Thu, 3 Dec 2020 21:18:16 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Feedback Thomas
>
> src/hotspot/share/runtime/os.cpp line 1869:
> 
>> 1867:     return round_down_power_of_2(v2);
>> 1868:   }
>> 1869:   return 0;
> 
> In next_larger you use the for:
> if (v2 == 0) {
>   return 0;
> }
> return ...
> I think it would be nice to be consistent between the two functions.

> Just a few drive-by comment: Could PagesizeSet be named PageSizeSet. The rest of the code separates the word, so I think that name would be better. The same for some of the parameters that are named pagesize.
> 
> Also, I think you could have called the class PageSizes. I don't think the fact that this is a 'set' matters for the usages of the class. It also makes the variable name match the type name, which I guess is an indication that it wouldn't be such a bad name:
> 
> ```
> static PageSizes        _page_sizes;`
> ```
> 
> vs
> 
> ```
>   static PagesizeSet        _page_sizes;
> ```

Hi Stefan,

thanks for your review! I adapted most of your suggestions.

Cheers, Thomas

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

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


More information about the hotspot-dev mailing list