RFR: 8334230: Optimize C2 classes layout

[Aleksey Shipilev]

On Mon, 24 Jun 2024 15:53:24 GMT, Neethu Prasad <nprasad at openjdk.org> wrote:

> **Notes**
> 
> Rearrange C2 class fields to optimize footprint.
> 
> 
> **Verification**
> 
> 1. Ran tier2_compiler, hotspot_compiler, tier 1 & tier 2 tests.
> 2. Ran pahole on 64 bit machine post re-ordering and verified that there are no holes / reduction in total bytes.
> 
> | Class | Size | Cachelines | Sum Members | Holes | Sum holes | Last Cacheline | Padding |
> | ----- | ----- | ---------- | --------------- | ----- | ---------- | --------------- | -------- |
> | ArrayPointer | 56 -> 48 | 1 -> 1 | 45 -> 0 | 2 ->  0 | 11 -> 0  | 56 bytes -> 48 | 0 -> 3 |
> | CallJavaNode | 152 -> 144 | 3 -> 3 | 12 -> 0 | 1 ->  0 | 5 -> 0  | 24 bytes -> 16 | 7 -> 4 |
> | C2Access | 56 -> 48 | 1-> 1 | 42 -> 0 | 1 ->  0 | 7 -> 0  | 56 bytes -> 48 | 7 -> 6 |
> | VectorSet| 32 -> 24 | 1-> 1 | 24 -> 0 | 1 ->  0 | 8 -> 0  | 32 bytes -> 24 | 1 -> 1 |
> 
> class ArrayPointer {
> 	const class Node  *        _pointer;             /*     0     8 */
> 	const class Node  *        _base;                /*     8     8 */
> 	const jlong                _constant_offset;     /*    16     8 */
> 	const class Node  *        _int_offset;          /*    24     8 */
> 	const class GrowableArray<Node*>  * _other_offsets; /*    32     8 */
> 	const jint                 _int_offset_shift;    /*    40     4 */
> 	const bool                 _is_valid;            /*    44     1 */
> public:
> 
> 
> 	/* size: 48, cachelines: 1, members: 7 */
> 	/* padding: 3 */
> 	/* last cacheline: 48 bytes */
> };
> 
> 
> 
> class CallJavaNode : public CallNode {
> public:
> 
> 	/* class CallNode            <ancestor>; */      /*     0   128 */
> protected:
> 
> 	/* --- cacheline 2 boundary (128 bytes) --- */
> 	class ciMethod *           _method;              /*   128     8 */
> 	bool                       _optimized_virtual;   /*   136     1 */
> 	bool                       _method_handle_invoke; /*   137     1 */
> 	bool                       _override_symbolic_info; /*   138     1 */
> 	bool                       _arg_escape;          /*   139     1 */
> public:
> 
> protected:
> 
> public:
> 
> 
> 	/* size: 144, cachelines: 3, members: 6 */
> 	/* padding: 4 */
> 	/* last cacheline: 16 bytes */
> 
> 	/* BRAIN FART ALERT! 144 bytes != 12 (member bytes) + 0 (member bits) + 0 (byte holes) + 0 (bit holes), diff = 1024 bits */
> };
> 
> 
> 
> class C2Access : public StackObj {
> public:
> 
> 	/* class StackObj            <ancestor>; */      /*     0     0 */
> 
> 	/* XXX last struct has 1 byte of padding */
> 
> 	int ()(void) * *           _vptr.C2Access;       /*     0     8 */
> protected:
> 
> 	DecoratorSet               _decorators;          /*     8  ...

Looks fine, but I think we want to keep argument list in current order. Unless there is a good reason to change it, and I just don't see it?

src/hotspot/share/gc/shared/c2/barrierSetC2.hpp line 115:

> 113: public:
> 114:   C2Access(DecoratorSet decorators,
> 115:            Node* base, C2AccessValuePtr& addr, BasicType type) :

I think it would be cleaner to leave the argument order alone here, and only change the field order. This would guarantee we do not change anything in APIs, which simplifies future changes and backports.

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

Marked as reviewed by shade (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19861#pullrequestreview-2198664777
PR Review Comment: https://git.openjdk.org/jdk/pull/19861#discussion_r1691054895