Skip to content

Close padding hole in BuiltinFuncDescriptor#24175

Open
richardleach wants to merge 1 commit intoPerl:bleadfrom
richardleach:BuiltinFuncDescriptor-6bytehole
Open

Close padding hole in BuiltinFuncDescriptor#24175
richardleach wants to merge 1 commit intoPerl:bleadfrom
richardleach:BuiltinFuncDescriptor-6bytehole

Conversation

@richardleach
Copy link
Contributor

The pahole tool, on AMD64, shows that this commit converts:

struct BuiltinFuncDescriptor {
        const char  *              name;                 /*     0     8 */
        U16                        since_ver;            /*     8     2 */

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

        XSUBADDR_t                 xsub;                 /*    16     8 */
        OP *                       (*checker)(OP *, GV *, SV *); /*    24     8 */
        IV                         ckval;                /*    32     8 */
        _Bool                      is_experimental;      /*    40     1 */

        /* size: 48, cachelines: 1, members: 6 */
        /* sum members: 35, holes: 1, sum holes: 6 */
        /* padding: 7 */
        /* last cacheline: 48 bytes */
};

to

struct BuiltinFuncDescriptor {
        const char  *              name;                 /*     0     8 */
        U16                        since_ver;            /*     8     2 */
        _Bool                      is_experimental;      /*    10     1 */

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

        XSUBADDR_t                 xsub;                 /*    16     8 */
        OP *                       (*checker)(OP *, GV *, SV *); /*    24     8 */
        IV                         ckval;                /*    32     8 */

        /* size: 40, cachelines: 1, members: 6 */
        /* sum members: 35, holes: 1, sum holes: 5 */
        /* last cacheline: 40 bytes */
};

  • This set of changes does not require a perldelta entry.

The pahole tool, on AMD64, shows that this commit converts:
```
struct BuiltinFuncDescriptor {
        const char  *              name;                 /*     0     8 */
        U16                        since_ver;            /*     8     2 */

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

        XSUBADDR_t                 xsub;                 /*    16     8 */
        OP *                       (*checker)(OP *, GV *, SV *); /*    24     8 */
        IV                         ckval;                /*    32     8 */
        _Bool                      is_experimental;      /*    40     1 */

        /* size: 48, cachelines: 1, members: 6 */
        /* sum members: 35, holes: 1, sum holes: 6 */
        /* padding: 7 */
        /* last cacheline: 48 bytes */
};
```

to

```
struct BuiltinFuncDescriptor {
        const char  *              name;                 /*     0     8 */
        U16                        since_ver;            /*     8     2 */
        _Bool                      is_experimental;      /*    10     1 */

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

        XSUBADDR_t                 xsub;                 /*    16     8 */
        OP *                       (*checker)(OP *, GV *, SV *); /*    24     8 */
        IV                         ckval;                /*    32     8 */

        /* size: 40, cachelines: 1, members: 6 */
        /* sum members: 35, holes: 1, sum holes: 5 */
        /* last cacheline: 40 bytes */
};
```
Copy link
Contributor

@leonerd leonerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

{ "floor", SHORTVER(5,39), false, &XS_builtin_func1_scalar, &ck_builtin_func1, OP_FLOOR },
{ "is_tainted", SHORTVER(5,39), false, &XS_builtin_func1_scalar, &ck_builtin_func1, OP_IS_TAINTED },
{ "trim", SHORTVER(5,39), false, &XS_builtin_trim, &ck_builtin_func1, 0 },
{ "stringify", NO_BUNDLE, true, &XS_builtin_func1_scalar, &ck_builtin_func1, OP_STRINGIFY },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to add a space after the true, to line it all up, like is_bool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants