RFR: 8151413: os::allocation_granularity/page_size and friends return signed values [v6]

Calvin Cheung ccheung at openjdk.org
Fri Jan 20 17:33:36 UTC 2023


On Fri, 20 Jan 2023 09:41:28 GMT, Afshin Zafari <duke at openjdk.org> wrote:

>> ### Description
>> os::allocation_granularity/page_size and friends return signed values
>> 
>> ### Patch
>> - Type of `vm_page_size` and `vm_allocation_granularity` members of `OSInfo` class and their wrappers in `os` class changed to `size_t`
>> - Initial value of them changed from -1 to 0.
>> - In setters, checking for *set only once* condition is updated accordingly (comparing with 0 instead of -1). Also, checking the argument be positive is removed.
>> - Equal to 0 (instead of `<= 0` ) is used to check if calling setters failed.
>> - All `(size_t)` casting of getters removed.
>> - In arithmetic and negation operations, the operand related to the getters casted to `(int)`. Otherwise, the Windows builds complain.
>> - Explicitly casted to `(int)` where `jint` needed.
>> - In `<T, A> align_up(T size, A alignment)`, assignment of variables of type `A` to type `T` (i.e., `T t = (A) a;`) should be safe. `T : size_t` and `A : int` won't compile. Fixed appropriately.
>> - `"%d"` format-flags replaced with `SIZE_FORMAT`.
>> - Type of `CompilerToVM::Data::vm_page_size` changed to `size_t`.
>> 
>> ### Test
>> tier1-5: all green, except an unrelated fail for whom a bug is already created.
>> job-id: afshin-8151413-20230117-1255-40910454
>
> Afshin Zafari has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
> 
>  - Merge 'master'
>  - 8151413: os::allocation_granularity/page_size and friends return signed values
>  - 8151413: os::allocation_granularity/page_size and friends return signed values
>  - 8151413: os::allocation_granularity/page_size and friends return signed values
>  - 8151413: os::allocation_granularity/page_size and friends return signed values
>  - 8151413: os::allocation_granularity/page_size and friends return signed values

Looks good except for a few copyright issues.

src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp line 2:

> 1: /*
> 2:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.

I'm not sure if we need to add this line for such a small change in the file.

src/hotspot/share/runtime/osInfo.hpp line 2:

> 1: /*
> 2:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.

You need to preserve the original year. It should be `2022, 2023,`

test/hotspot/gtest/utilities/test_globalDefinitions.cpp line 2:

> 1: /*
> 2:  * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.

Same as above, need to preserve the original year.

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

Marked as reviewed by ccheung (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12091


More information about the shenandoah-dev mailing list