[Mesa-dev] [PATCH] mesa: detect inefficient buffer use and report through debug output

[Ilia Mirkin]

On Mon, Dec 7, 2015 at 8:42 PM, Brian Paul <brianp at vmware.com> wrote:
> When a buffer is created with GL_STATIC_DRAW, its contents should not
> be changed frequently.  But that's exactly what one application I'm
> debugging does.  This patch adds code to try to detect inefficient
> buffer use in a couple places.  The GL_ARB_debug_output mechanism is
> used to report the issue.
>
> NVIDIA's driver detects these sort of things too.
>
> Other types of inefficient buffer use could also be detected in the
> future.
> ---
>  src/mesa/main/bufferobj.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/mesa/main/mtypes.h    |  4 ++++
>  2 files changed, 59 insertions(+)
>
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index f985982..6bc1b5e 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -51,6 +51,34 @@
>
>
>  /**
> + * We count the number of buffer modification calls to check for
> + * inefficient buffer use.  This is the number of such calls before we
> + * issue a warning.
> + */
> +#define BUFFER_WARNING_CALL_COUNT 4
> +
> +
> +/**
> + * Helper to warn of possible performance issues, such as frequently
> + * updating a buffer created with GL_STATIC_DRAW.
> + */
> +static void
> +buffer_usage_warning(struct gl_context *ctx, const char *fmt, ...)
> +{
> +   va_list args;
> +   GLuint msg_id = 0;

This needs to be wrapped in a macro, with a 'static' id (at each macro
invocation), otherwise a fresh id will get generated each time this is
called, which is presumably not desirable. Same as what I did with
pipe_debug_message/_pipe_debug_message.

[I know you already pushed this... that's how I noticed... but should
still get fixed.]

  -ilia

> +
> +   va_start(args, fmt);
> +   _mesa_gl_vdebug(ctx, &msg_id,
> +                   MESA_DEBUG_SOURCE_API,
> +                   MESA_DEBUG_TYPE_PERFORMANCE,
> +                   MESA_DEBUG_SEVERITY_MEDIUM,
> +                   fmt, args);
> +   va_end(args);
> +}
> +
> +
> +/**
>   * Used as a placeholder for buffer objects between glGenBuffers() and
>   * glBindBuffer() so that glIsBuffer() can work correctly.
>   */
> @@ -1677,6 +1705,21 @@ _mesa_buffer_sub_data(struct gl_context *ctx, struct gl_buffer_object *bufObj,
>     if (size == 0)
>        return;
>
> +   bufObj->NumSubDataCalls++;
> +
> +   if ((bufObj->Usage == GL_STATIC_DRAW ||
> +        bufObj->Usage == GL_STATIC_COPY) &&
> +       bufObj->NumSubDataCalls >= BUFFER_WARNING_CALL_COUNT) {
> +      /* If the application declared the buffer as static draw/copy or stream
> +       * draw, it should not be frequently modified with glBufferSubData.
> +       */
> +      buffer_usage_warning(ctx,
> +                           "using %s(buffer %u, offset %u, size %u) to "
> +                           "update a %s buffer",
> +                           func, bufObj->Name, offset, size,
> +                           _mesa_enum_to_string(bufObj->Usage));
> +   }
> +
>     bufObj->Written = GL_TRUE;
>
>     assert(ctx->Driver.BufferSubData);
> @@ -2384,6 +2427,18 @@ _mesa_map_buffer_range(struct gl_context *ctx,
>        return NULL;
>     }
>
> +   if (access & GL_MAP_WRITE_BIT) {
> +      bufObj->NumMapBufferWriteCalls++;
> +      if ((bufObj->Usage == GL_STATIC_DRAW ||
> +           bufObj->Usage == GL_STATIC_COPY) &&
> +          bufObj->NumMapBufferWriteCalls >= BUFFER_WARNING_CALL_COUNT) {
> +         buffer_usage_warning(ctx,
> +                              "using %s(buffer %u, offset %u, length %u) to "
> +                              "update a %s buffer",
> +                              func, bufObj->Name, offset, length,
> +                              _mesa_enum_to_string(bufObj->Usage));
> +      }
> +   }
>
>     assert(ctx->Driver.MapBufferRange);
>     map = ctx->Driver.MapBufferRange(ctx, offset, length, access, bufObj,
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 1eb1e21..de54169 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1275,6 +1275,10 @@ struct gl_buffer_object
>     GLboolean Immutable; /**< GL_ARB_buffer_storage */
>     gl_buffer_usage UsageHistory; /**< How has this buffer been used so far? */
>
> +   /** Counters used for buffer usage warnings */
> +   GLuint NumSubDataCalls;
> +   GLuint NumMapBufferWriteCalls;
> +
>     struct gl_buffer_mapping Mappings[MAP_COUNT];
>  };
>
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev