RFR: 8367485: os::physical_memory is broken in 32-bit JVMs when running on 64-bit OSes [v13]
Anton Artemov
duke at openjdk.org
Thu Sep 25 10:24:27 UTC 2025
On Thu, 25 Sep 2025 09:55:56 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
>> Anton Artemov has updated the pull request incrementally with two additional commits since the last revision:
>>
>> - 8367485: Fixed new line after typedef.
>> - 8367485: Fixed comment in global definitions.
>
> src/hotspot/share/utilities/globalDefinitions.hpp line 422:
>
>> 420:
>> 421: //----------------------------------------------------------------------------------------------------
>> 422: // Type defenition for memory functions
>
> There's a typo here defenition > definition. With that said, this comment mimics the comments for the sections above and below:
>
> // VM type definitions
> // Java type definitions
>
> but we don't need a new section, so I think we should not follow those patterns. Instead it would be good to have a motivation for having this typedef. Something that explains that the amount of physical memory doesn't necessarily fit in a 32-bit size_t for 32-bit JVMs and that we have this typedef to mark the places where we have to deal with that quirk as long as we support 32-bit JVMs.
Thanks, I addressed this by adding an extended comment. No new section added as suggested.
> src/hotspot/share/utilities/globalDefinitions.hpp line 423:
>
>> 421: //----------------------------------------------------------------------------------------------------
>> 422: // Type defenition for memory functions
>> 423: typedef uint64_t physical_memory_size_type;
>
> When adding code make sure have a blank line between the new code and the surrounding code.
Line added.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27335#discussion_r2378568022
PR Review Comment: https://git.openjdk.org/jdk/pull/27335#discussion_r2378569525
More information about the hotspot-dev
mailing list