[Mesa-dev] [PATCH] mesa/marshal: add custom marshalling for glNamedBuffer(Sub)Data

[Timothy Arceri]

Please try the series from Marek which reduces the batch size [1], the 
reduced size helps reduce the impact of syncs. MARSHAL_MAX_CMD_SIZE is 
also greatly reduced to help reduce thrashing the cache so its possible 
this patch won't be as effective anymore. However you might not even 
need it.

[1] https://lists.freedesktop.org/archives/mesa-dev/2017-June/160329.html

On 25/06/17 02:59, Grigori Goronzy wrote:
> These entry points are used by Alien Isolation and caused
> synchronization with glthread. The async marshalling implementation
> is similar to glBuffer(Sub)Data.
> 
> Results in an approximately 6x drop in glthread synchronizations and a
> ~30% FPS jump in Alien Isolation (Medium preset, Athlon 860K, RX 480).
> 
> This does not care about the EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD special
> case like the Buffer(Sub)Data marshalling functions.
> ---
> I'm not a fan of the code duplication and I'll try to address that in
> further changes to glthread/marshalling, but the improvement is so
> noticeable that I'd like to share it. Alien Isolation is now playable on
> my system while it wasn't before.
> 
>   src/mapi/glapi/gen/ARB_direct_state_access.xml |   4 +-
>   src/mesa/main/marshal.c                        | 108 +++++++++++++++++++++++++
>   src/mesa/main/marshal.h                        |  18 +++++
>   3 files changed, 128 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> index cb24d79..d3d2246 100644
> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> @@ -61,14 +61,14 @@
>         <param name="flags" type="GLbitfield" />
>      </function>
>   
> -   <function name="NamedBufferData">
> +   <function name="NamedBufferData" marshal="custom">
>         <param name="buffer" type="GLuint" />
>         <param name="size" type="GLsizeiptr" />
>         <param name="data" type="const GLvoid *" />
>         <param name="usage" type="GLenum" />
>      </function>
>   
> -   <function name="NamedBufferSubData" no_error="true">
> +   <function name="NamedBufferSubData" no_error="true" marshal="custom">
>         <param name="buffer" type="GLuint" />
>         <param name="offset" type="GLintptr" />
>         <param name="size" type="GLsizeiptr" />
> diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
> index 4840f32..1fddf8e 100644
> --- a/src/mesa/main/marshal.c
> +++ b/src/mesa/main/marshal.c
> @@ -408,6 +408,114 @@ _mesa_marshal_BufferSubData(GLenum target, GLintptr offset, GLsizeiptr size,
>      }
>   }
>   
> +/* NamedBufferData: marshalled asynchronously */
> +struct marshal_cmd_NamedBufferData
> +{
> +   struct marshal_cmd_base cmd_base;
> +   GLuint name;
> +   GLsizei size;
> +   GLenum usage;
> +   /* Next size bytes are GLubyte data[size] */
> +};
> +
> +void
> +_mesa_unmarshal_NamedBufferData(struct gl_context *ctx,
> +                                const struct marshal_cmd_NamedBufferData *cmd)
> +{
> +   const GLuint name = cmd->name;
> +   const GLsizei size = cmd->size;
> +   const GLenum usage = cmd->usage;
> +   const void *data = (const void *) (cmd + 1);
> +
> +   CALL_NamedBufferData(ctx->CurrentServerDispatch,
> +                      (name, size, data, usage));
> +}
> +
> +void GLAPIENTRY
> +_mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr size,
> +                              const GLvoid * data, GLenum usage)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   size_t cmd_size = sizeof(struct marshal_cmd_NamedBufferData) + size;
> +
> +   debug_print_marshal("NamedBufferData");
> +   if (unlikely(size < 0)) {
> +      _mesa_glthread_finish(ctx);
> +      _mesa_error(ctx, GL_INVALID_VALUE, "NamedBufferData(size < 0)");
> +      return;
> +   }
> +
> +   if (buffer > 0 && cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> +      struct marshal_cmd_NamedBufferData *cmd =
> +         _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_NamedBufferData,
> +                                         cmd_size);
> +      cmd->name = buffer;
> +      cmd->size = size;
> +      cmd->usage = usage;
> +      char *variable_data = (char *) (cmd + 1);
> +      memcpy(variable_data, data, size);
> +      _mesa_post_marshal_hook(ctx);
> +   } else {
> +      _mesa_glthread_finish(ctx);
> +      CALL_NamedBufferData(ctx->CurrentServerDispatch,
> +                         (buffer, size, data, usage));
> +   }
> +}
> +
> +/* NamedBufferSubData: marshalled asynchronously */
> +struct marshal_cmd_NamedBufferSubData
> +{
> +   struct marshal_cmd_base cmd_base;
> +   GLuint name;
> +   GLintptr offset;
> +   GLsizei size;
> +   /* Next size bytes are GLubyte data[size] */
> +};
> +
> +void
> +_mesa_unmarshal_NamedBufferSubData(struct gl_context *ctx,
> +                                   const struct marshal_cmd_NamedBufferSubData *cmd)
> +{
> +   const GLuint name = cmd->name;
> +   const GLintptr offset = cmd->offset;
> +   const GLsizei size = cmd->size;
> +   const void *data = (const void *) (cmd + 1);
> +
> +   CALL_NamedBufferSubData(ctx->CurrentServerDispatch,
> +                      (name, offset, size, data));
> +}
> +
> +void GLAPIENTRY
> +_mesa_marshal_NamedBufferSubData(GLuint buffer, GLintptr offset,
> +                                 GLsizeiptr size, const GLvoid * data)
> +{
> +   GET_CURRENT_CONTEXT(ctx);
> +   size_t cmd_size = sizeof(struct marshal_cmd_NamedBufferSubData) + size;
> +
> +   debug_print_marshal("NamedBufferSubData");
> +   if (unlikely(size < 0)) {
> +      _mesa_glthread_finish(ctx);
> +      _mesa_error(ctx, GL_INVALID_VALUE, "NamedBufferSubData(size < 0)");
> +      return;
> +   }
> +
> +   if (buffer > 0 && cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> +      struct marshal_cmd_NamedBufferSubData *cmd =
> +         _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_NamedBufferSubData,
> +                                         cmd_size);
> +      cmd->name = buffer;
> +      cmd->offset = offset;
> +      cmd->size = size;
> +      char *variable_data = (char *) (cmd + 1);
> +      memcpy(variable_data, data, size);
> +      _mesa_post_marshal_hook(ctx);
> +   } else {
> +      _mesa_glthread_finish(ctx);
> +      CALL_NamedBufferSubData(ctx->CurrentServerDispatch,
> +                         (buffer, offset, size, data));
> +   }
> +}
> +
>   /* ClearBufferfv: marshalled asynchronously */
>   struct marshal_cmd_ClearBufferfv
>   {
> diff --git a/src/mesa/main/marshal.h b/src/mesa/main/marshal.h
> index ddfa834..999c75e 100644
> --- a/src/mesa/main/marshal.h
> +++ b/src/mesa/main/marshal.h
> @@ -180,6 +180,8 @@ struct marshal_cmd_Flush;
>   struct marshal_cmd_BindBuffer;
>   struct marshal_cmd_BufferData;
>   struct marshal_cmd_BufferSubData;
> +struct marshal_cmd_NamedBufferData;
> +struct marshal_cmd_NamedBufferSubData;
>   struct marshal_cmd_ClearBufferfv;
>   
>   void
> @@ -228,6 +230,22 @@ _mesa_marshal_BufferSubData(GLenum target, GLintptr offset, GLsizeiptr size,
>                               const GLvoid * data);
>   
>   void
> +_mesa_unmarshal_NamedBufferData(struct gl_context *ctx,
> +                                const struct marshal_cmd_NamedBufferData *cmd);
> +
> +void GLAPIENTRY
> +_mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr size,
> +                              const GLvoid * data, GLenum usage);
> +
> +void
> +_mesa_unmarshal_NamedBufferSubData(struct gl_context *ctx,
> +                                   const struct marshal_cmd_NamedBufferSubData *cmd);
> +
> +void GLAPIENTRY
> +_mesa_marshal_NamedBufferSubData(GLuint buffer, GLintptr offset, GLsizeiptr size,
> +                                 const GLvoid * data);
> +
> +void
>   _mesa_unmarshal_ClearBufferfv(struct gl_context *ctx,
>                                 const struct marshal_cmd_ClearBufferfv *cmd);
>   
>