Re: [Mesa-dev] [PATCH] mesa: detect inefficient buffer use and report through debug output
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
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
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, &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@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
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, &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]; }; LGTM. Reviewed-by: Jose Fonseca ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev