RFR: 8342011: Conditionally compile ReservedHeapSpace compressed heap support [v2]
Stefan Karlsson
stefank at openjdk.org
Wed Oct 16 08:04:11 UTC 2024
On Wed, 16 Oct 2024 07:54:48 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> Please review this change to ReservedHeapSpace to conditionally include the
>> support for compressed oops only in 64bit builds. This code is unused and
>> inapplicable in 32bit builds. Further, it contains code that is problematic
>> in a 32bit build. See the JBS issue for details.
>>
>> Testing: mach5 tier1, GHA testing
>
> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
>
> improve conditionalization
Changes requested by stefank (Reviewer).
src/hotspot/share/memory/virtualspace.cpp line 646:
> 644: #ifndef _LP64
> 645: NOT_LP64(ShouldNotReachHere();)
> 646: #else
Suggestion:
#ifndef _LP64
ShouldNotReachHere();
#else
With that said, I think it would be nicer to invert the code:
#ifdef _LP64
... All the code we typically care of up front
#else
... Code we typically ignore
ShouldNotReachHere();
#endif
src/hotspot/share/memory/virtualspace.hpp line 138:
> 136:
> 137: // Compressed oop support is not relevant in 32bit builds.
> 138: #ifdef _LP64
Suggestion:
// Compressed oop support is not relevant in 32bit builds.
#ifdef _LP64
The `#endif // _LP64 below have blank lines before and after, so it seems more consistent to also have it here.`
-------------
PR Review: https://git.openjdk.org/jdk/pull/21484#pullrequestreview-2371525505
PR Review Comment: https://git.openjdk.org/jdk/pull/21484#discussion_r1802558014
PR Review Comment: https://git.openjdk.org/jdk/pull/21484#discussion_r1802560318
More information about the hotspot-runtime-dev
mailing list