RFR: 8334220: Optimize Klass layout after JDK-8180450

[Coleen Phillimore]

On Sat, 29 Jun 2024 19:58:23 GMT, Xiaolong Peng <xpeng 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;
>                                static const int SECONDARY_SUPERS_TABLE_MASK;
>                                static const...

I have a couple of questions about this and a request.  Thanks.

src/hotspot/share/oops/klass.hpp line 166:

> 164:   uintx    _bitmap;
> 165: 
> 166:   static uint8_t compute_hash_slot(Symbol* s);

We don't usually put functions in with nonstatic member declarations.  Since you moved hash_slot, can you move this function to the first private section where hash_insert is?

Where you moved hash_slot looks fine.  Doesn't look like it will have negative cache effects, unless it needs to be on a cache line with _bitmap?

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?

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

Changes requested by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19958#pullrequestreview-2154109976
PR Review Comment: https://git.openjdk.org/jdk/pull/19958#discussion_r1662698438
PR Review Comment: https://git.openjdk.org/jdk/pull/19958#discussion_r1662701347