[Mesa-dev] [PATCH] st/mesa: In update_samplers(), clear all samplers at once.

[Keith Whitwell]

On Mon, 2011-03-21 at 19:55 +0100, Tilman Sauerbeck wrote:
> Keith Whitwell [2011-03-21 18:43]:
> > On Mon, 2011-03-21 at 19:28 +0100, Tilman Sauerbeck wrote:
> > > Signed-off-by: Tilman Sauerbeck <tilman at code-monkey.de>
> > > ---
> > > 
> > > update_samplers() showed up in a profile of Heroes of Newerth;
> > > this patch pushes it down the profile by ~3%.
> > > 
> > > Does this seem plausible?
> > > 
> > >  src/mesa/state_tracker/st_atom_sampler.c |    5 +++--
> > >  1 files changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/mesa/state_tracker/st_atom_sampler.c b/src/mesa/state_tracker/st_atom_sampler.c
> > > index 474cbd5..4374ac1 100644
> > > --- a/src/mesa/state_tracker/st_atom_sampler.c
> > > +++ b/src/mesa/state_tracker/st_atom_sampler.c
> > > @@ -129,12 +129,13 @@ update_samplers(struct st_context *st)
> > >  
> > >     st->state.num_samplers = 0;
> > >  
> > > +   memset(st->state.samplers, 0, st->ctx->Const.MaxTextureImageUnits *
> > > +          sizeof(struct pipe_sampler_state));
> > > +
> > 
> > At a glance, could the memset be moved up another couple of lines and
> > changed to:
> > 
> >  memset(st->state.samplers, 0, st->state.num_samplers * sizeof(struct pipe_sampler_state));
> 
> I wondered about this, too. Consider the case where the sampler state is
> undefined when update_samplers() is called though -- then we might end
> up with partially uninitialized samplers, no?

In theory as long as the array started off zeroed, it should just work &
be a decent win.  Adding an assert or two might help catch cases where
we are wrong about this.

If you don't do that, I wonder if there is really a gain here -- it
might be that the original memset was inlined (because it has a known
size) and the new version just converts that to a non-inlined memset
with an unknown size?

As it stands, I don't see why this generates such a big improvement.

Keith