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

[Stefan Karlsson]

On Thu, 3 Dec 2020 13:59:11 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Hi,
>> 
>> may I please have reviews for the following very small improvement:
>> 
>> While discussing JDK-8243315 [1], and aiming to make planned changes like JDK-8256155 [2] easier:
>> 
>> size_t os::_page_sizes[os::page_sizes_max];
>> 
>> is an array used to keep all page sizes the hotspot can use. It is sorted by size and filled in at initialization time.
>> 
>> Coding dealing with this can be simplified by making this a set (which is very easy since all page sizes are power-2-values so they lend themselves nicely to a bitmap).
>> 
>> That has the following advantages:
>> - makes adding new sizes simple since we do not have to re-sort the array. Coding is easier to read too.
>> - it makes it possible to encode a set of page sizes in one number, so we can hand a set-of-page-sizes around as a value.
>> 
>> -----
>> 
>> The patch adds a new class, os::PagesizeSet, which is a bitmap containing page sizes. It adds gtests for this class. It replaces the old os::_page_sizes with an object of that class. It also removes an unused function.
>> 
>> 
>> Testing:
>> - nightlies at SAP
>> - manual tests with UseLargePages
>> - the new gtests
>> - gh actions (with x86 still failing because of unrelated issues)
>> 
>> Thank you,
>> 
>> Thomas
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8243315
>> [2] https://bugs.openjdk.java.net/browse/JDK-8256155
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Feedback Thomas

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;

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

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