[PATCH] glamor: Don't unbind the GL_ARRAY_BUFFER in glamor_put_vbo_space().

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> Mesa's implementation of glBindBuffer() checks to see if the supplied
> buffer name happens to be already bound.  If so, it returns immediately,
> skipping the hash table lookup (mapping the GLuint buffer handle to the
> actual struct), the associated locking, and reference counting.
>
> Glamor uses a single VBO for everything - operations reserve space in
> the VBO via glamor_get_vbo_space(); if it fills up, we replace it with
> a fresh buffer (which is then used for everything).  Ideally, Glamor
> would almost always hit the above optimization.
>
> Instead, it never hits it.  Today, glamor_get_vbo_space() binds the VBO,
> and glamor_put_vbo_space() explicitly unbinds it.  This means that our
> VBO is never bound on a get operation, so we pay the hash table cost
> every time we call glamor_get_vbo_space().  Every get/put also alter
> reference counts, so we pay that cost twice over.
>
> This patch removes the explicit unbind, which makes us hit the fast path
> on every get/put (except the rare ones which create a new buffer).
>
> When running a fullscreen GL application under xf86-video-modesetting
> (which doesn't do page flipping yet), glamor_get_vbo_space() accounted
> for 4.24% of the CPU instructions in each glamor_copy_fbo_fbo_draw()
> operation.  With this patch, it became only 0.87%.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Eric Anholt <eric at anholt.net>
> ---
>  glamor/glamor_vbo.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/glamor/glamor_vbo.c b/glamor/glamor_vbo.c
> index e906101..5d7c55f 100644
> --- a/glamor/glamor_vbo.c
> +++ b/glamor/glamor_vbo.c
> @@ -150,8 +150,6 @@ glamor_put_vbo_space(ScreenPtr screen)
>          glBufferData(GL_ARRAY_BUFFER, glamor_priv->vbo_offset,
>                       glamor_priv->vb, GL_DYNAMIC_DRAW);
>      }
> -
> -    glBindBuffer(GL_ARRAY_BUFFER, 0);
>  }

OK, I've been trying this patch (sort of -- a variant that readds this
unbind to the pointer-using draw paths and which can be found on
"glamor-testing-patch-from-ken" of my tree), and I'm getting a
consistent hit in Xephyr with copypixwin10: -3.70065% +/- 1.73153%
(n=72), and it's persisted across a few runs of experiments trying to
figure out what's going on.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150204/24f271e3/attachment.sig>