[Mesa-dev] [PATCH] llvmpipe: Optimize new fs state setup

[Keith Whitwell]

On Wed, 2011-06-29 at 13:19 -0400, Adam Jackson wrote:
> Perversely, do this by eliminating the comparison between stored and
> current fs state.  On ipers, a perf trace showed try_update_scene_state
> using 31% of a CPU, and 98% of that was in 'repz cmpsb', ie, the memcmp.
> Taking that out takes try_update_scene_state down to 6.5% of the
> profile; more importantly, ipers goes from 10 to 14fps and gears goes
> from 790 to 830fps.

Some of the motivation for that memcpy is about keeping the memory usage
of the binned scene from exploding and forcing unnecessary flushes on
more complex apps.

I wonder if there is a way to improve the dirty flag handling to avoid
ending up in that memcpy so often?


Note that freeglut is probably dominating your gears numbers by trying
to reinitialize your SpaceBall device (I don't have one either) on every
swapbuffers.

http://lists.freedesktop.org/archives/mesa-dev/2011-February/005599.html


Keith


> Signed-off-by: Adam Jackson <ajax at redhat.com>
> ---
>  src/gallium/drivers/llvmpipe/lp_setup.c |   61 ++++++++++++++-----------------
>  1 files changed, 27 insertions(+), 34 deletions(-)
> 
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
> index cbe06e5..9118db5 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> @@ -839,42 +839,35 @@ try_update_scene_state( struct lp_setup_context *setup )
>        setup->dirty |= LP_SETUP_NEW_FS;
>     }
>  
> -
>     if (setup->dirty & LP_SETUP_NEW_FS) {
> -      if (!setup->fs.stored ||
> -          memcmp(setup->fs.stored,
> -                 &setup->fs.current,
> -                 sizeof setup->fs.current) != 0)
> -      {
> -         struct lp_rast_state *stored;
> -         uint i;
> -         
> -         /* The fs state that's been stored in the scene is different from
> -          * the new, current state.  So allocate a new lp_rast_state object
> -          * and append it to the bin's setup data buffer.
> -          */
> -         stored = (struct lp_rast_state *) lp_scene_alloc(scene, sizeof *stored);
> -         if (!stored) {
> -            assert(!new_scene);
> -            return FALSE;
> -         }
> +      struct lp_rast_state *stored;
> +      uint i;
> +      
> +      /* The fs state that's been stored in the scene is different from
> +       * the new, current state.  So allocate a new lp_rast_state object
> +       * and append it to the bin's setup data buffer.
> +       */
> +      stored = (struct lp_rast_state *) lp_scene_alloc(scene, sizeof *stored);
> +      if (!stored) {
> +         assert(!new_scene);
> +         return FALSE;
> +      }
>  
> -         memcpy(stored,
> -                &setup->fs.current,
> -                sizeof setup->fs.current);
> -         setup->fs.stored = stored;
> -         
> -         /* The scene now references the textures in the rasterization
> -          * state record.  Note that now.
> -          */
> -         for (i = 0; i < Elements(setup->fs.current_tex); i++) {
> -            if (setup->fs.current_tex[i]) {
> -               if (!lp_scene_add_resource_reference(scene,
> -                                                    setup->fs.current_tex[i],
> -                                                    new_scene)) {
> -                  assert(!new_scene);
> -                  return FALSE;
> -               }
> +      memcpy(stored,
> +             &setup->fs.current,
> +             sizeof setup->fs.current);
> +      setup->fs.stored = stored;
> +      
> +      /* The scene now references the textures in the rasterization
> +       * state record.  Note that now.
> +       */
> +      for (i = 0; i < Elements(setup->fs.current_tex); i++) {
> +         if (setup->fs.current_tex[i]) {
> +            if (!lp_scene_add_resource_reference(scene,
> +                                                 setup->fs.current_tex[i],
> +                                                 new_scene)) {
> +               assert(!new_scene);
> +               return FALSE;
>              }
>           }
>        }