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

2015-12-09 Thread Ilia Mirkin 
On Mon, Dec 7, 2015 at 8:42 PM, Brian Paul  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, _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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-12-09 Thread Ilia Mirkin 
On Wed, Dec 9, 2015 at 5:23 PM, Brian Paul  wrote:
> On 12/09/2015 11:43 AM, Ilia Mirkin wrote:
>>
>> On Mon, Dec 7, 2015 at 8:42 PM, Brian Paul  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.
>
>
> Is the macro required, or can I just pass a pointer to a static GLuint from
> each call site?  I took a quick stab at the macro but I'm having trouble
> with the varargs stuff.

The macro is not required... but you can copy the one I used for
pipe_debug_message. Passing in a different static variable from each
callsite would work too -- that's precisely what the macro would be
doing in the first place.

  -ilia
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-12-09 Thread Brian Paul 

On 12/09/2015 11:43 AM, Ilia Mirkin wrote:

On Mon, Dec 7, 2015 at 8:42 PM, Brian Paul  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.


Is the macro required, or can I just pass a pointer to a static GLuint 
from each call site?  I took a quick stab at the macro but I'm having 
trouble with the varargs stuff.


-Brian

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-12-08 Thread Jose Fonseca 

On 08/12/15 01:42, Brian Paul 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;
+
+   va_start(args, fmt);
+   _mesa_gl_vdebug(ctx, _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];
  };




LGTM.

Reviewed-by: Jose Fonseca 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-12-07 Thread Brian Paul 
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;
+
+   va_start(args, fmt);
+   _mesa_gl_vdebug(ctx, _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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev