[PATCH] input: calloc minimization for xi2mask_new

Peter Hutterer peter.hutterer at who-t.net
Wed Oct 2 03:07:08 PDT 2013


On Wed, Sep 11, 2013 at 11:44:35AM -0400, Jasper St. Pierre wrote:
> Though I'd love to look into the gnome-shell issue more (changing to the
> overview should not call XIGrabKey/XIUngrabKey a ton of times), the
> handling in the server here was ridiculous and this is obviously an
> improvement.
> 
> Reviewed-by: Jasper St. Pierre <jstpierre at mecheye.net>

merged, thanks.

Cheers,
   Peter


> On Tue, Sep 10, 2013 at 2:18 PM, Adam Jackson <ajax at redhat.com> wrote:
> 
> > There's no reason to do this as (nmasks + 2) callocs, and it's a
> > surprisingly hot path.  Turns out you hit this ~once per passive grab,
> > and you do a few bajillion passive grab changes every time you enter or
> > leave the overview in gnome-shell.  According to a callgrind of Xorg
> > with gnome-shell-perf-tool run against it:
> >
> > Ir before: 721437275
> > Ir after:  454227086
> >
> > Signed-off-by: Adam Jackson <ajax at redhat.com>
> > ---
> >  dix/inpututils.c | 30 +++++++++++++-----------------
> >  1 file changed, 13 insertions(+), 17 deletions(-)
> >
> > diff --git a/dix/inpututils.c b/dix/inpututils.c
> > index 9e38e17..a10a7c7 100644
> > --- a/dix/inpututils.c
> > +++ b/dix/inpututils.c
> > @@ -960,8 +960,15 @@ XI2Mask *
> >  xi2mask_new_with_size(size_t nmasks, size_t size)
> >  {
> >      int i;
> > +    int alloc_size;
> > +    unsigned char *cursor;
> > +    XI2Mask *mask;
> >
> > -    XI2Mask *mask = calloc(1, sizeof(*mask));
> > +    alloc_size = sizeof(struct _XI2Mask)
> > +              + nmasks * sizeof(unsigned char *)
> > +              + nmasks * size;
> > +
> > +    mask = calloc(1, alloc_size);
> >
> >      if (!mask)
> >          return NULL;
> > @@ -969,20 +976,14 @@ xi2mask_new_with_size(size_t nmasks, size_t size)
> >      mask->nmasks = nmasks;
> >      mask->mask_size = size;
> >
> > -    mask->masks = calloc(mask->nmasks, sizeof(*mask->masks));
> > -    if (!mask->masks)
> > -        goto unwind;
> > +    mask->masks = (unsigned char **)(mask + 1);
> > +    cursor = (unsigned char *)(mask + 1) + nmasks * sizeof(unsigned char
> > *);
> >
> > -    for (i = 0; i < mask->nmasks; i++) {
> > -        mask->masks[i] = calloc(1, mask->mask_size);
> > -        if (!mask->masks[i])
> > -            goto unwind;
> > +    for (i = 0; i < nmasks; i++) {
> > +        mask->masks[i] = cursor;
> > +       cursor += size;
> >      }
> >      return mask;
> > -
> > - unwind:
> > -    xi2mask_free(&mask);
> > -    return NULL;
> >  }
> >
> >  /**
> > @@ -1003,14 +1004,9 @@ xi2mask_new(void)
> >  void
> >  xi2mask_free(XI2Mask **mask)
> >  {
> > -    int i;
> > -
> >      if (!(*mask))
> >          return;
> >
> > -    for (i = 0; (*mask)->masks && i < (*mask)->nmasks; i++)
> > -        free((*mask)->masks[i]);
> > -    free((*mask)->masks);
> >      free((*mask));
> >      *mask = NULL;
> >  }
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > xorg-devel at lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: http://lists.x.org/mailman/listinfo/xorg-devel
> >
> 
> 
> 
> -- 
>   Jasper

> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel



More information about the xorg-devel mailing list