[lworld] RFR: 8373787: [lworld] InstanceKlass::size mixes bytes and words
Paul Hübner
phubner at openjdk.org
Tue Dec 16 12:55:19 UTC 2025
On Tue, 16 Dec 2025 12:26:42 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
> The following function in `InstanceKlass` is supposed to return size in words:
>
> static int size(int vtable_length, int itable_length,
> int nonstatic_oop_map_size,
> bool is_interface,
> bool is_inline_type) {
> return align_metadata_size(header_size() +
> vtable_length +
> itable_length +
> nonstatic_oop_map_size +
> (is_interface ? (int)sizeof(Klass*)/wordSize : 0) +
> (is_inline_type ? (int)sizeof(InlineKlassFixedBlock) : 0));
> }
>
> but `sizeof(InlineKlassFixedBlock)` returns a size in bytes. This adds 640 bytes instead of 80 bytes to all InlineKlasses.
>
> This can be seen by running `jcmd <pid> VM.classes` to check the size of the InlineKlass of Integer before and after the fix for this:
>
> Before:
>
> 0x00000ffc0024d260 167 fully_initialized WS java.lang.Integer
>
> After:
>
> 0x000001800024d260 97 fully_initialized WS java.lang.Integer
>
>
> That is in words. So this bug added an extra `(167 - 97) * 8 == 560` bytes.
>
> I've so far only done some local smoke testing with TEST=hotspot_valhalla
This is a good catch! I think it would be good to stress test these changes with (App)CDS/AOT and COH on/off, possibly in conjunction. Judging by the fact that the RefArrayKlass assertion was kind of weak this whole time, I wouldn't be surprised if this unearths more CDS issues.
src/hotspot/share/oops/flatArrayKlass.hpp line 95:
> 93:
> 94: // sizing
> 95: static int header_size() { return sizeof(FlatArrayKlass) / wordSize; }
Curiously enough we actually got this right for `RefArrayKlass::header_size()` and I'm guessing the following assertion didn't actually do anything meaningful:
assert(RefArrayKlass::header_size() <= InstanceKlass::header_size(),
"array klasses must be same size as InstanceKlass");
src/hotspot/share/oops/instanceKlass.hpp line 1050:
> 1048: nonstatic_oop_map_size +
> 1049: (is_interface ? (int)sizeof(Klass*) / wordSize : 0) +
> 1050: (is_inline_type ? (int)sizeof(InlineKlassFixedBlock) / wordSize : 0));
Do we have an `x / wordSize` helper function anywhere? Too much inline arithmetic makes it hard to maintain, imo.
-------------
Marked as reviewed by phubner (Committer).
PR Review: https://git.openjdk.org/valhalla/pull/1804#pullrequestreview-3582934819
PR Review Comment: https://git.openjdk.org/valhalla/pull/1804#discussion_r2623156322
PR Review Comment: https://git.openjdk.org/valhalla/pull/1804#discussion_r2623158498
More information about the valhalla-dev
mailing list