RFR: 8334220: Optimize Klass layout after JDK-8180450

Xiaolong Peng xpeng at openjdk.org
Tue Jul 2 16:53:22 UTC 2024


On Tue, 2 Jul 2024 14:58:34 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Hi all, 
>>     This PR is created to optimize the layout of Klass in hotspot, after JDK-8180450 the layout of Klsss seems broken, there are 3 holes, they are caused by alignment issue introduced by the 1 byte ```_hash_slot```. 
>> 
>> 
>> (gdb) ptype /ox Klass
>> /* offset      |    size */  type = class Klass : public Metadata {
>>                              public:
>>                                static const uint KLASS_KIND_COUNT;
>>                              protected:
>> /* 0x000c      |  0x0004 */    jint _layout_helper;
>> /* 0x0010      |  0x0004 */    const enum Klass::KlassKind _kind;
>> /* 0x0014      |  0x0004 */    jint _modifier_flags;
>> /* 0x0018      |  0x0004 */    juint _super_check_offset;
>> /* XXX  4-byte hole      */
>> /* 0x0020      |  0x0008 */    class Symbol *_name;
>> /* 0x0028      |  0x0008 */    class Klass *_secondary_super_cache;
>> /* 0x0030      |  0x0008 */    class Array<Klass*> *_secondary_supers;
>> /* 0x0038      |  0x0040 */    class Klass *_primary_supers[8];
>> /* 0x0078      |  0x0008 */    class OopHandle {
>>                                  private:
>> /* 0x0078      |  0x0008 */        class oop *_obj;
>> 
>>                                    /* total size (bytes):    8 */
>>                                } _java_mirror;
>> /* 0x0080      |  0x0008 */    class Klass *_super;
>> /* 0x0088      |  0x0008 */    class Klass * volatile _subklass;
>> /* 0x0090      |  0x0008 */    class Klass * volatile _next_sibling;
>> /* 0x0098      |  0x0008 */    class Klass *_next_link;
>> /* 0x00a0      |  0x0008 */    class ClassLoaderData *_class_loader_data;
>> /* 0x00a8      |  0x0008 */    uintx _bitmap;
>> /* 0x00b0      |  0x0001 */    uint8_t _hash_slot;
>> /* XXX  3-byte hole      */
>> /* 0x00b4      |  0x0004 */    int _vtable_len;
>> /* 0x00b8      |  0x0004 */    class AccessFlags {
>>                                  private:
>> /* 0x00b8      |  0x0004 */        jint _flags;
>> 
>>                                    /* total size (bytes):    4 */
>>                                } _access_flags;
>> /* XXX  4-byte hole      */
>> /* 0x00c0      |  0x0008 */    traceid _trace_id;
>>                              private:
>> /* 0x00c8      |  0x0002 */    s2 _shared_class_path_index;
>> /* 0x00ca      |  0x0002 */    u2 _shared_class_flags;
>> /* 0x00cc      |  0x0004 */    int _archived_mirror_index;
>>                              public:
>>                                static const int SECONDARY_SUPERS_TABLE_SIZE;
>>                         ...
>
> src/hotspot/share/oops/klass.hpp line 176:
> 
>> 174:   JFR_ONLY(DEFINE_TRACE_ID_FIELD;)
>> 175:   uint8_t  _hash_slot;
>> 176:   DEFINE_PAD_MINUS_SIZE(1, 4, sizeof(uint8_t)); //3 bytes padding after 1 byte _hash_slot for better layout
> 
> How does this help? Doesn't the compiler add this padding?

Compiler doesn't seem to add padding, there will be two smaller holes after _hash_slot, here is what I got from gdb:

/* 0x00b8      |  0x0001 */    uint8_t _hash_slot;
                             private:
/* XXX  1-byte hole      */
/* 0x00ba      |  0x0002 */    s2 _shared_class_path_index;
/* 0x00bc      |  0x0002 */    u2 _shared_class_flags;
/* XXX  2-byte hole      */
/* 0x00c0      |  0x0004 */    int _archived_mirror_index;


I think we can remove it to avoid confusion, I doesn't really change the cache line, here is output from ```pahole``` with cacheline boundaries:
With padding:

protected:

	jint                       _layout_helper;       /*     8     4 */
	const enum KlassKind       _kind;                /*    12     4 */
	jint                       _modifier_flags;      /*    16     4 */
	juint                      _super_check_offset;  /*    20     4 */
	class Symbol *             _name;                /*    24     8 */
	class Klass *              _secondary_super_cache; /*    32     8 */
	class Array<Klass*> *      _secondary_supers;    /*    40     8 */
	class Klass *              _primary_supers[8];   /*    48    64 */
	/* --- cacheline 1 boundary (64 bytes) was 48 bytes ago --- */
	class OopHandle           _java_mirror;          /*   112     8 */
	class Klass *              _super;               /*   120     8 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	volatile class Klass *     _subklass;            /*   128     8 */
	volatile class Klass *     _next_sibling;        /*   136     8 */
	class Klass *              _next_link;           /*   144     8 */
	class ClassLoaderData *    _class_loader_data;   /*   152     8 */
	uintx                      _bitmap;              /*   160     8 */
	int                        _vtable_len;          /*   168     4 */
	class AccessFlags         _access_flags;         /*   172     4 */
	traceid                    _trace_id;            /*   176     8 */
	uint8_t                    _hash_slot;           /*   184     1 */
	char                       _pad_buf1[3];         /*   185     3 */
	s2                         _shared_class_path_index; /*   188     2 */
	u2                         _shared_class_flags;  /*   190     2 */
	/* --- cacheline 3 boundary (192 bytes) --- */
	int                        _archived_mirror_index; /*   192     4 */


w/o padding:

protected:

	jint                       _layout_helper;       /*     8     4 */
	const enum KlassKind       _kind;                /*    12     4 */
	jint                       _modifier_flags;      /*    16     4 */
	juint                      _super_check_offset;  /*    20     4 */
	class Symbol *             _name;                /*    24     8 */
	class Klass *              _secondary_super_cache; /*    32     8 */
	class Array<Klass*> *      _secondary_supers;    /*    40     8 */
	class Klass *              _primary_supers[8];   /*    48    64 */
	/* --- cacheline 1 boundary (64 bytes) was 48 bytes ago --- */
	class OopHandle           _java_mirror;          /*   112     8 */
	class Klass *              _super;               /*   120     8 */
	/* --- cacheline 2 boundary (128 bytes) --- */
	volatile class Klass *     _subklass;            /*   128     8 */
	volatile class Klass *     _next_sibling;        /*   136     8 */
	class Klass *              _next_link;           /*   144     8 */
	class ClassLoaderData *    _class_loader_data;   /*   152     8 */
	uintx                      _bitmap;              /*   160     8 */
	int                        _vtable_len;          /*   168     4 */
	class AccessFlags         _access_flags;         /*   172     4 */
	traceid                    _trace_id;            /*   176     8 */
	uint8_t                    _hash_slot;           /*   184     1 */

	/* XXX 1 byte hole, try to pack */

	s2                         _shared_class_path_index; /*   186     2 */
	u2                         _shared_class_flags;  /*   188     2 */

	/* XXX 2 bytes hole, try to pack */

	/* --- cacheline 3 boundary (192 bytes) --- */
	int                        _archived_mirror_index; /*   192     4 */

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19958#discussion_r1662866957


More information about the hotspot-dev mailing list