[PATCH 10/22] udev: Use shared NoopDDA to utilize code cache better

[Mark Kettenis]

> Date: Thu, 30 Dec 2010 11:32:18 +0200
> From: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> 
> On 29/12/10 21:19 +0100, ext Mark Kettenis wrote:
> > > From: Pauli <ext-pauli.nieminen at nokia.com>
> > > Date: Wed, 29 Dec 2010 21:27:22 +0200
> > > 
> > > From: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> > > 
> > > Calling function that is in code cache is order of magnitude faster. In
> > > arm non-cached simple function takes about 1us while cached function
> > > takes max 200ns.
> > 
> > > diff --git a/config/udev.c b/config/udev.c
> > > index 496bfbf..393723c 100644
> > > --- a/config/udev.c
> > > +++ b/config/udev.c
> > > @@ -253,11 +253,6 @@ wakeup_handler(pointer data, int err, pointer read_mask)
> > >      }
> > >  }
> > >  
> > > -static void
> > > -block_handler(pointer data, struct timeval **tv, pointer read_mask)
> > > -{
> > > -}
> > > -
> > >  int
> > >  config_udev_init(void)
> > >  {
> > > @@ -290,7 +285,7 @@ config_udev_init(void)
> > >      }
> > >      udev_enumerate_unref(enumerate);
> > >  
> > > -    RegisterBlockAndWakeupHandlers(block_handler, wakeup_handler, NULL);
> > > +    RegisterBlockAndWakeupHandlers((BlockHandlerProcPtr)NoopDDA, wakeup_handler, NULL);
> > 
> > YUCK!  Obfuscating code like this is sooo wrong.  I don't think
> > optimization hacks like this that only really matter for hardware with
> > last-century sized caches belong in the Xorg tree.
> 
> Obfuscating? Can you explain how this obfuscates code?

Sure.  The code adds casts.  Casts are almost always bad.  Here they
are "bad" since calling a function through an incompatible pointer
yields undefined behaviour according to the C standard.  Now in
practice, any sane implementation of C will probably do the sane thing
in this case.  But it still makes me somewhat unhappy to add those casts.

> I tough that NoopDDA is standard way of assign empty function to any
> callback. It is already used in most places but these weren't using it.

OK, I wasn't aware of this.  It does indeed look as if NoopDDA() is
used in this fashion in a lot more places.  Sorry, I'm not too
familliar with the bits of code where it is used a lot.  Objection
withdrawn.

On the other hand...

> IMO it is less clear if we have everywhere random empty function just for
> broken BlockAndWakeupHandlers. But I guess fixing the API might also option.

...I think Daniel's suggested diff makes a lot of sense and would be a
cleaner way to fix things.