On Fri, Apr 28, 2017 at 05:25:31AM -0400, Frediano Ziglio wrote:
> >
> > This is intended to hold the fields that only char-device.c has a use
> > for, but for now this only adds the boilerplate for it, the next commit
> > will move the relevant field there.
> >
> > Signed-off-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> > ---
> > server/char-device.c | 5 +++++
> > server/char-device.h | 3 +++
> > 2 files changed, 8 insertions(+)
> >
> > diff --git a/server/char-device.c b/server/char-device.c
> > index d32593209..fa137e416 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -31,6 +31,9 @@
> > #define RED_CHAR_DEVICE_WAIT_TOKENS_TIMEOUT 30000
> > #define MAX_POOL_SIZE (10 * 64 * 1024)
> >
> > +struct RedCharDeviceWriteBufferPrivate {
> > +};
> > +
> > typedef struct RedCharDeviceClient RedCharDeviceClient;
> > struct RedCharDeviceClient {
> > RedCharDevice *dev;
> > @@ -144,6 +147,7 @@ static void
> > red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
> > return;
> >
> > free(buf->buf);
> > + free(buf->priv);
> > free(buf);
> > }
> >
> > @@ -540,6 +544,7 @@ static RedCharDeviceWriteBuffer
> > *__red_char_device_write_buffer_get(
> > dev->priv->cur_pool_size -= ret->buf_size;
> > } else {
> > ret = spice_new0(RedCharDeviceWriteBuffer, 1);
> > + ret->priv = spice_new0(RedCharDeviceWriteBufferPrivate, 1);
> > }
> >
> > spice_assert(!ret->buf_used);
>
> Would not be better if this is always allocated here to have a
> public part and a private one after the public to avoid the
> indirection penalty?
>
> So kind of
>
> struct RedCharDeviceWriteBufferPrivate {
> RedCharDeviceWriteBuffer base;
> .. additional fields here ...
> };
>
> ...
>
> ret = &spice_new0(RedCharDeviceWriteBufferPrivate, 1)->base;
I considered playing tricks like
ret = spice_malloc0(sizeof(RedCharDeviceWriteBuffer) +
sizeof(RedCharDeviceWriteBufferPrivate));
ret->priv = &ret[1];
to allocate it, but was not sure it was worth it over the simple version
in this patch.
>
> > diff --git a/server/char-device.h b/server/char-device.h
> > index 7854086c3..9bfd7c656 100644
> > --- a/server/char-device.h
> > +++ b/server/char-device.h
> > @@ -146,6 +146,7 @@ GType red_char_device_get_type(void) G_GNUC_CONST;
> > * */
> >
> > /* buffer that is used for writing to the device */
> > +typedef struct RedCharDeviceWriteBufferPrivate
> > RedCharDeviceWriteBufferPrivate;
> > typedef struct RedCharDeviceWriteBuffer {
> > int origin;
> > RedClient *client; /* The client that sent the message to the device.
> > @@ -156,6 +157,8 @@ typedef struct RedCharDeviceWriteBuffer {
> > uint32_t buf_used;
> > uint32_t token_price;
> > uint32_t refs;
> > +
> > + RedCharDeviceWriteBufferPrivate *priv;
> > } RedCharDeviceWriteBuffer;
> >
> > void red_char_device_reset_dev_instance(RedCharDevice *dev,
>
> I would put client field (patch 3/6) as first to reduce structure size on
> 64 bit.
I kept the same order as in the initial struct. However, moving things
around does not seem to make a difference size-wise, padding is added to the
end rather than just after the first member.
struct RedCharDeviceWriteBufferPrivate {
RedClient * client; /* 0 8 */
WriteBufferOrigin origin; /* 8 4 */
uint32_t token_price; /* 12 4 */
uint32_t refs; /* 16 4 */
/* size: 24, cachelines: 1, members: 4 */
/* padding: 4 */
/* last cacheline: 24 bytes */
};
struct RedCharDeviceWriteBufferPrivate {
WriteBufferOrigin origin; /* 0 4 */
/* XXX 4 bytes hole, try to pack */
RedClient * client; /* 8 8 */
uint32_t token_price; /* 16 4 */
uint32_t refs; /* 20 4 */
/* size: 24, cachelines: 1, members: 4 */
/* sum members: 20, holes: 1, sum holes: 4 */
/* last cacheline: 24 bytes */
};
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel