RFR: 8334231: Optimize MethodData layout

[Christian Hagedorn]

On Thu, 4 Jul 2024 00:08:35 GMT, Xiaolong Peng <xpeng at openjdk.org> wrote:

> Hi all,
>      This PR is a part of https://bugs.openjdk.org/browse/JDK-8334227 to optimize Hotspot C++ class layouts, this one is for the layout of  MethodData. Here is the original layout from `pahole`:
> 
> class MethodData : public Metadata {
> public:
> 
> 	/* class Metadata            <ancestor>; */      /*     0     0 */
> 
> 	/* XXX 8 bytes hole, try to pack */
> 
> 	class Method *             _method;              /*     8     8 */
> 	int                        _size;                /*    16     4 */
> 	int                        _hint_di;             /*    20     4 */
> 	class Mutex               _extra_data_lock;      /*    24   104 */
> 	/* --- cacheline 2 boundary (128 bytes) --- */
> 	class CompilerCounters    _compiler_counters;    /*   128    80 */
> 	/* --- cacheline 3 boundary (192 bytes) was 16 bytes ago --- */
> 	intx                       _eflags;              /*   208     8 */
> 	intx                       _arg_local;           /*   216     8 */
> 	intx                       _arg_stack;           /*   224     8 */
> 	intx                       _arg_returned;        /*   232     8 */
> 	int                        _creation_mileage;    /*   240     4 */
> 	class InvocationCounter   _invocation_counter;   /*   244     4 */
> 	class InvocationCounter   _backedge_counter;     /*   248     4 */
> 	int                        _invocation_counter_start; /*   252     4 */
> 	/* --- cacheline 4 boundary (256 bytes) --- */
> 	int                        _backedge_counter_start; /*   256     4 */
> 	uint                       _tenure_traps;        /*   260     4 */
> 	int                        _invoke_mask;         /*   264     4 */
> 	int                        _backedge_mask;       /*   268     4 */
> 	short int                  _num_loops;           /*   272     2 */
> 	short int                  _num_blocks;          /*   274     2 */
> 	enum WouldProfile          _would_profile;       /*   276     4 */
> 	int                        _jvmci_ir_size;       /*   280     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	class FailedSpeculation *  _failed_speculations; /*   288     8 */
> 	int                        _data_size;           /*   296     4 */
> 	int                        _parameters_type_data_di; /*   300     4 */
> 	int                        _exception_handler_data_di; /*   304     4 */
> 
> 	/* XXX 4 bytes hole, try to pack */
> 
> 	intptr_t                   _data[1];             /*   312     8 */
> 
> 	/* size: 320, cachelines: 5, members: 27 */
> 	/* sum members: 304, holes: 3, sum holes: 16 */
> };
> 
> 
> There are 3 holes ...

That's a fair point. It's sometimes tricky to find the boundary between trivial and non-trivial. Here I thought about it for a bit but it looked trivial (but I understand that you could think about it differently) and I was the second reviewer. Nevertheless, as you pointed out, I agree that we should probably restrict the use of this rule to the really urgent issues (most are not). My default is usually to just wait 24h for normal issues even for changes that are marked as trivial just to give everyone a chance to have a look. Sometimes, you still get some valuable feedback that you could have missed otherwise when integrating early.

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

PR Comment: https://git.openjdk.org/jdk/pull/20019#issuecomment-2209179254