RFR: 8334231: Optimize MethodData layout

[Thomas Stuefe]

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 ...

I don't think these "Optimize XXX layouts" should be marked as trivial, and I worry that we overuse the trivial rule. It circumvents the second reviewer as well as the 24hr rule, which both are necessary safeties. Especially in the wake of the xz fiasco.

"Trivial" is usually reserved for either changes that need very quick reaction (e.g. reasonably simple build errors that require immediate fixing because everyone's CI is standing still) or things that are painfully obvious in being trivial, e.g. comment changes. Memory layout changes are neither urgent nor really trivial enough IMHO.

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

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