[Mesa-dev] [PATCH] mesa/marshal: add custom marshalling forglNamedBuffer(Sub)Data
Marc Dietrich
marvin24 at gmx.de
Mon Jun 26 13:11:14 UTC 2017
Hi,
Am Montag, 26. Juni 2017, 01:17:23 CEST schrieb Timothy Arceri:
> On 25/06/17 18:31, Grigori Goronzy wrote:
> > On 2017-06-25 02:37, Timothy Arceri wrote:
> >> 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.
> >
> > Sorry, I forgot to mention, the 30% improvement measured is with this
> > patch on top of Marek's series compared to just Marek's series. That
> > series alone is improving glthread with Alien Isolation as well, but I
> > didn't measure exactly how much. It wouldn't surprise me if it is in
> > the 40-50% region with both, though.
>
> Ok, thanks for confirming. I've fixed the indentation issues, reworded
> the commit message and pushed. Thanks for the patch :)
unfortunately, this change broke vmware/vmplayer here (bisected). Windows
guest on linux host. Sig 11 in SVGA driver. All good if mesa_glthread=false.
Marc
>
> > Best regards
> > Grigori
> >
> >> [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);
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170626/b7e7732f/attachment-0001.sig>
More information about the mesa-dev
mailing list