On Mon, 30 Mar 2026 23:21:13 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Chris Plummer has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> use if/else
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/utilities/HeapHprofBinWriter.java
> line 1210:
>
>> 1208: // as a null reference.
>> 1209: writeObjectID(null);
>> 1210: } else {
>
> Nit: I'd suggest to use `else if` here, so the indentation is not increased.
Ok. I made that change.
Related to this I've been staring at the OopField vs NarrowOopField cast for a
few days now, trying to convince myself that the two separate paths are not
needed and the check for isCompressedOopsEnabled() is not needed.
NarrowOopField is a subclass of OopField. It's enough just to do the OopField
cast. I just made this change and it works with and with compressed oops
enabled. I'd like to fix this in mainline, but then someone will need to deal
with a merge conflict in valhalla. I think I'll just leave it as is.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2280#discussion_r3013061779