[Xcb] Question: Why are structs not serialized elementwise in the autogenerated C cocde?

[Christoph Reimann]

Hi & thanks again for the advice!

> I suspect it was never done this way for performance reasons. Larger
> iovecs mean more dcache usage, more setting means more icache usage, and
> processing all the extra elements means more CPU usage.
>
> That said, I doubt anyone has ever measured the difference. Perhaps you
> could measure and let us know.
>

I did 10.000 xcb_create_window() calls with current libxcb and
modified (see below [0] for the generated code) - after running it for
several times, there seems to be no difference considering the
accuracy of 'time', conforming with Jamey's post (the tests did some
harm to my X server though - Xorg is keeping my CPU quite busy now and
my legs warm...):

latest libxcb-git
real	0m2.334s
user	0m0.080s
sys	0m0.300s

modified libxcb:
real	0m2.235s
user	0m0.100s
sys	0m0.330s

> Alternatively, you could automatically join adjacent fields where
> possible, allowing you to insert variable fields without changing the
> output in the case where there are no variable fields. I'm not sure how
> much code would be needed to do that in c_client.py.
>

Not sure if I got this right - do you suggest serializing only those
structs elemtent-wise where fixed- and variable-sized fields are
mixed? (Which would be no problem in c_client.py.)
By serializing everything field-wise I can now (in theory as untested)
handle protocol definitions that include things like lists of variable
sized members and mixed variable/fixed types - at the expense of
increased autogenerated code base:

xproto.c: 14471 lines
modified xproto.c: 17061 lines

So anyway - for the code in c_client.py it is somewhat simpler to
handle the serializing consistently - but then the code changes amount
only to about 80 lines.

> That said, I'd be much happier if all padding was explicitly specified
> in the .xml files, and none was ever automagically inserted by any of
> the generators. See, for example, commit 0d62c1d0 (and surrounding
> commit series) where we added some of the missing padding. It wouldn't
> surprise me to discover that we'd missed some.

Again unsure if I understood correctly (sorry :) - if the xml spec
would contain the padding, then I should be able to calculate in
c_client.py (by simply adding up the sizes of the fixed fields)
whether the specified padding is complete?

Have a nice evening,

Christoph


[0] modified xcb_create_window request:

xcb_void_cookie_t
xcb_create_window (xcb_connection_t *c  /**< */,
                   uint8_t           depth  /**< */,
                   xcb_window_t      wid  /**< */,
                   xcb_window_t      parent  /**< */,
                   int16_t           x  /**< */,
                   int16_t           y  /**< */,
                   uint16_t          width  /**< */,
                   uint16_t          height  /**< */,
                   uint16_t          border_width  /**< */,
                   uint16_t          _class  /**< */,
                   xcb_visualid_t    visual  /**< */,
                   uint32_t          value_mask  /**< */,
                   const uint32_t   *value_list  /**< */)
{
    unsigned int xcb_unpadded = 0;
    static const xcb_protocol_request_t xcb_req = {
        /* count */ 14,
        /* ext */ 0,
        /* opcode */ XCB_CREATE_WINDOW,
        /* isvoid */ 1
    };

    struct iovec xcb_parts[16];
    xcb_void_cookie_t xcb_ret;

    /* xcb_create_window_request_t.depth */
    xcb_parts[2].iov_base = (char *) &depth;
    xcb_parts[2].iov_len  = sizeof(uint8_t);
    xcb_unpadded += sizeof(uint8_t);
    /* xcb_create_window_request_t.wid */
    xcb_parts[3].iov_base = (char *) &wid;
    xcb_parts[3].iov_len  = sizeof(xcb_window_t);
    xcb_unpadded += sizeof(xcb_window_t);
    /* xcb_create_window_request_t.parent */
    xcb_parts[4].iov_base = (char *) &parent;
    xcb_parts[4].iov_len  = sizeof(xcb_window_t);
    xcb_unpadded += sizeof(xcb_window_t);
    /* xcb_create_window_request_t.x */
    xcb_parts[5].iov_base = (char *) &x;
    xcb_parts[5].iov_len  = sizeof(int16_t);
    xcb_unpadded += sizeof(int16_t);
    /* xcb_create_window_request_t.y */
    xcb_parts[6].iov_base = (char *) &y;
    xcb_parts[6].iov_len  = sizeof(int16_t);
    xcb_unpadded += sizeof(int16_t);
    /* xcb_create_window_request_t.width */
    xcb_parts[7].iov_base = (char *) &width;
    xcb_parts[7].iov_len  = sizeof(uint16_t);
    xcb_unpadded += sizeof(uint16_t);
    /* xcb_create_window_request_t.height */
    xcb_parts[8].iov_base = (char *) &height;
    xcb_parts[8].iov_len  = sizeof(uint16_t);
    xcb_unpadded += sizeof(uint16_t);
    /* xcb_create_window_request_t.border_width */
    xcb_parts[9].iov_base = (char *) &border_width;
    xcb_parts[9].iov_len  = sizeof(uint16_t);
    xcb_unpadded += sizeof(uint16_t);
    /* xcb_create_window_request_t._class */
    xcb_parts[10].iov_base = (char *) &_class;
    xcb_parts[10].iov_len  = sizeof(uint16_t);
    xcb_unpadded += sizeof(uint16_t);
    /* xcb_create_window_request_t.visual */
    xcb_parts[11].iov_base = (char *) &visual;
    xcb_parts[11].iov_len  = sizeof(xcb_visualid_t);
    xcb_unpadded += sizeof(xcb_visualid_t);
    /* xcb_create_window_request_t.value_mask */
    xcb_parts[12].iov_base = (char *) &value_mask;
    xcb_parts[12].iov_len  = sizeof(uint32_t);
    xcb_unpadded += sizeof(uint32_t);
    xcb_parts[13].iov_base = 0;
    xcb_parts[13].iov_len = -xcb_unpadded & 3;
    /* value_list */
    xcb_parts[14].iov_base = (char *) value_list;
    xcb_parts[14].iov_len  = xcb_popcount(value_mask) * sizeof(uint32_t);
    xcb_unpadded = xcb_parts[14].iov_len;
    xcb_parts[15].iov_base = 0;
    xcb_parts[15].iov_len = -xcb_unpadded & 3;

    xcb_ret.sequence = xcb_send_request(c, 0, xcb_parts + 2, &xcb_req);
    return xcb_ret;
}