Re: Speed up transaction completion faster after many relations areaccessed in a transaction

Hi,

On 2019-02-18 19:24:54 -0500, Tom Lane wrote:
> Yeah, but if we want to rearrange the members into an illogical order
> to save some space, we should do that independently of this patch ---

Sure, we should do that. I don't buy the "illogical" bit, just moving
hashcode up to after tag isn't more or less logical, and saves most of
the padding, and moving the booleans to the end isn't better/worse
either.

You always bring up that argument. While I agree that sometimes the most
optimal ordering can be less natural, I think most of the time it vastly
overstates how intelligent the original ordering was. Often new elements
were either just added iteratively without consideration for padding, or
the attention to padding was paid in 32bit times.

I don't find

struct LOCALLOCK {
        LOCALLOCKTAG               tag;                  /*     0    20 */
        uint32                     hashcode;             /*    20     4 */
        LOCK *                     lock;                 /*    24     8 */
        PROCLOCK *                 proclock;             /*    32     8 */
        int64                      nLocks;               /*    40     8 */
        int                        numLockOwners;        /*    48     4 */
        int                        maxLockOwners;        /*    52     4 */
        LOCALLOCKOWNER *           lockOwners;           /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        _Bool                      holdsStrongLockCount; /*    64     1 */
        _Bool                      lockCleared;          /*    65     1 */

        /* size: 72, cachelines: 2, members: 10 */
        /* padding: 6 */
        /* last cacheline: 8 bytes */
};

less clear than

struct LOCALLOCK {
        LOCALLOCKTAG               tag;                  /*     0    20 */

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

        LOCK *                     lock;                 /*    24     8 */
        PROCLOCK *                 proclock;             /*    32     8 */
        uint32                     hashcode;             /*    40     4 */

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

        int64                      nLocks;               /*    48     8 */
        _Bool                      holdsStrongLockCount; /*    56     1 */
        _Bool                      lockCleared;          /*    57     1 */

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

        int                        numLockOwners;        /*    60     4 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        int                        maxLockOwners;        /*    64     4 */

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

        LOCALLOCKOWNER *           lockOwners;           /*    72     8 */

        /* size: 80, cachelines: 2, members: 10 */
        /* sum members: 66, holes: 4, sum holes: 14 */
        /* last cacheline: 16 bytes */
};

but it's smaller (althoug there's plenty trailing space).


> and then the overhead of this patch would be even worse than 25%.

IDK, we, including you, very often make largely independent improvements
to make the cost of something else more palpable. Why's that not OK
here?  Especially because we're not comparing to an alternative where no
cost is added, keeping track of e.g. a running average of the hashtable
size isn't free either; nor does it help in the intermittent cases.

- Andres