[Mesa-dev] [PATCH] glthread: get rid of unmarshal dispatch enum/table
Nicolai Hähnle
nhaehnle at gmail.com
Fri Jun 30 13:27:05 UTC 2017
On 30.06.2017 02:29, Grigori Goronzy wrote:
> Use function pointers to identify the unmarshalling function, which
> is simpler and gets rid of a lot generated code.
>
> This removes an indirection and possibly results in a slight speedup
> as well.
The fact that it blows up cmd_base from 4 bytes to 16 bytes might result
in a slowdown. Marek's recent changes clearly indicated that looking at
memory behavior matters quite a bit for glthread. So I'm inclined to say
No on this unless you can demonstrate a consistent speedup.
Cheers,
Nicolai
> ---
> src/mapi/glapi/gen/Makefile.am | 4 --
> src/mapi/glapi/gen/gl_marshal.py | 36 ++--------------
> src/mapi/glapi/gen/gl_marshal_h.py | 86 --------------------------------------
> src/mesa/Android.gen.mk | 7 ----
> src/mesa/Makefile.sources | 1 -
> src/mesa/SConscript | 8 ----
> src/mesa/main/.gitignore | 1 -
> src/mesa/main/glthread.c | 9 +++-
> src/mesa/main/glthread.h | 2 -
> src/mesa/main/marshal.c | 19 ++++-----
> src/mesa/main/marshal.h | 14 +++----
> 11 files changed, 26 insertions(+), 161 deletions(-)
> delete mode 100644 src/mapi/glapi/gen/gl_marshal_h.py
>
> diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am
> index bd04519..62007a4 100644
> --- a/src/mapi/glapi/gen/Makefile.am
> +++ b/src/mapi/glapi/gen/Makefile.am
> @@ -76,7 +76,6 @@ EXTRA_DIST= \
> gl_genexec.py \
> gl_gentable.py \
> gl_marshal.py \
> - gl_marshal_h.py \
> gl_procs.py \
> gl_SPARC_asm.py \
> gl_table.py \
> @@ -297,9 +296,6 @@ $(MESA_DIR)/main/api_exec.c: gl_genexec.py apiexec.py $(COMMON)
> $(MESA_DIR)/main/marshal_generated.c: gl_marshal.py marshal_XML.py $(COMMON)
> $(PYTHON_GEN) $(srcdir)/gl_marshal.py -f $(srcdir)/gl_and_es_API.xml > $@
>
> -$(MESA_DIR)/main/marshal_generated.h: gl_marshal_h.py marshal_XML.py $(COMMON)
> - $(PYTHON_GEN) $(srcdir)/gl_marshal_h.py -f $(srcdir)/gl_and_es_API.xml > $@
> -
> $(MESA_DIR)/main/dispatch.h: gl_table.py $(COMMON)
> $(PYTHON_GEN) $(srcdir)/gl_table.py -f $(srcdir)/gl_and_es_API.xml -m remap_table > $@
>
> diff --git a/src/mapi/glapi/gen/gl_marshal.py b/src/mapi/glapi/gen/gl_marshal.py
> index efa4d9e..e71ede3 100644
> --- a/src/mapi/glapi/gen/gl_marshal.py
> +++ b/src/mapi/glapi/gen/gl_marshal.py
> @@ -34,7 +34,6 @@ header = """
> #include "dispatch.h"
> #include "glthread.h"
> #include "marshal.h"
> -#include "marshal_generated.h"
> """
>
>
> @@ -106,7 +105,7 @@ class PrintCode(gl_XML.gl_print_base):
>
> def print_async_dispatch(self, func):
> out('cmd = _mesa_glthread_allocate_command(ctx, '
> - 'DISPATCH_CMD_{0}, cmd_size);'.format(func.name))
> + '(unmarshal_func)_mesa_unmarshal_{0}, cmd_size);'.format(func.name))
> for p in func.fixed_params:
> if p.count:
> out('memcpy(cmd->{0}, {0}, {1});'.format(
> @@ -166,7 +165,7 @@ class PrintCode(gl_XML.gl_print_base):
> out('};')
>
> def print_async_unmarshal(self, func):
> - out('static inline void')
> + out('static void')
> out(('_mesa_unmarshal_{0}(struct gl_context *ctx, '
> 'const struct marshal_cmd_{0} *cmd)').format(func.name))
> out('{')
> @@ -205,6 +204,7 @@ class PrintCode(gl_XML.gl_print_base):
> else:
> out('variable_data += {0};'.format(p.size_string(False)))
>
> + out('debug_print_unmarshal("{0}");'.format(func.name))
> self.print_sync_call(func)
> out('}')
>
> @@ -276,35 +276,6 @@ class PrintCode(gl_XML.gl_print_base):
> out('')
> out('')
>
> - def print_unmarshal_dispatch_cmd(self, api):
> - out('size_t')
> - out('_mesa_unmarshal_dispatch_cmd(struct gl_context *ctx, '
> - 'const void *cmd)')
> - out('{')
> - with indent():
> - out('const struct marshal_cmd_base *cmd_base = cmd;')
> - out('switch (cmd_base->cmd_id) {')
> - for func in api.functionIterateAll():
> - flavor = func.marshal_flavor()
> - if flavor in ('skip', 'sync'):
> - continue
> - out('case DISPATCH_CMD_{0}:'.format(func.name))
> - with indent():
> - out('debug_print_unmarshal("{0}");'.format(func.name))
> - out(('_mesa_unmarshal_{0}(ctx, (const struct marshal_cmd_{0} *)'
> - ' cmd);').format(func.name))
> - out('break;')
> - out('default:')
> - with indent():
> - out('assert(!"Unrecognized command ID");')
> - out('break;')
> - out('}')
> - out('')
> - out('return cmd_base->cmd_size;')
> - out('}')
> - out('')
> - out('')
> -
> def print_create_marshal_table(self, api):
> out('struct _glapi_table *')
> out('_mesa_create_marshal_table(const struct gl_context *ctx)')
> @@ -338,7 +309,6 @@ class PrintCode(gl_XML.gl_print_base):
> async_funcs.append(func)
> elif flavor == 'sync':
> self.print_sync_body(func)
> - self.print_unmarshal_dispatch_cmd(api)
> self.print_create_marshal_table(api)
>
>
> diff --git a/src/mapi/glapi/gen/gl_marshal_h.py b/src/mapi/glapi/gen/gl_marshal_h.py
> deleted file mode 100644
> index 6e39148..0000000
> --- a/src/mapi/glapi/gen/gl_marshal_h.py
> +++ /dev/null
> @@ -1,86 +0,0 @@
> -#!/usr/bin/env python
> -
> -# Copyright (C) 2012 Intel Corporation
> -#
> -# Permission is hereby granted, free of charge, to any person obtaining a
> -# copy of this software and associated documentation files (the "Software"),
> -# to deal in the Software without restriction, including without limitation
> -# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> -# and/or sell copies of the Software, and to permit persons to whom the
> -# Software is furnished to do so, subject to the following conditions:
> -#
> -# The above copyright notice and this permission notice (including the next
> -# paragraph) shall be included in all copies or substantial portions of the
> -# Software.
> -#
> -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> -# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> -# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> -# IN THE SOFTWARE.
> -
> -import getopt
> -import gl_XML
> -import license
> -import marshal_XML
> -import sys
> -
> -
> -header = """
> -#ifndef MARSHAL_GENERATABLE_H
> -#define MARSHAL_GENERATABLE_H
> -"""
> -
> -footer = """
> -#endif /* MARSHAL_GENERATABLE_H */
> -"""
> -
> -
> -class PrintCode(gl_XML.gl_print_base):
> - def __init__(self):
> - super(PrintCode, self).__init__()
> -
> - self.name = 'gl_marshal_h.py'
> - self.license = license.bsd_license_template % (
> - 'Copyright (C) 2012 Intel Corporation', 'INTEL CORPORATION')
> -
> - def printRealHeader(self):
> - print header
> -
> - def printRealFooter(self):
> - print footer
> -
> - def printBody(self, api):
> - print 'enum marshal_dispatch_cmd_id'
> - print '{'
> - for func in api.functionIterateAll():
> - flavor = func.marshal_flavor()
> - if flavor in ('skip', 'sync'):
> - continue
> - print ' DISPATCH_CMD_{0},'.format(func.name)
> - print '};'
> -
> -
> -def show_usage():
> - print 'Usage: %s [-f input_file_name]' % sys.argv[0]
> - sys.exit(1)
> -
> -
> -if __name__ == '__main__':
> - file_name = 'gl_API.xml'
> -
> - try:
> - (args, trail) = getopt.getopt(sys.argv[1:], 'm:f:')
> - except Exception,e:
> - show_usage()
> -
> - for (arg,val) in args:
> - if arg == '-f':
> - file_name = val
> -
> - printer = PrintCode()
> -
> - api = gl_XML.parse_GL_API(file_name, marshal_XML.marshal_item_factory())
> - printer.Print(api)
> diff --git a/src/mesa/Android.gen.mk b/src/mesa/Android.gen.mk
> index ee2d1de..691fb3b 100644
> --- a/src/mesa/Android.gen.mk
> +++ b/src/mesa/Android.gen.mk
> @@ -41,7 +41,6 @@ sources := \
> main/remap_helper.h \
> main/get_hash.h \
> main/marshal_generated.c \
> - main/marshal_generated.h
>
> LOCAL_SRC_FILES := $(filter-out $(sources), $(LOCAL_SRC_FILES))
>
> @@ -110,12 +109,6 @@ $(intermediates)/main/marshal_generated.c: PRIVATE_XML := -f $(glapi)/gl_and_es_
> $(intermediates)/main/marshal_generated.c: $(dispatch_deps)
> $(call es-gen)
>
> -$(intermediates)/main/marshal_generated.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(glapi)/gl_marshal_h.py
> -$(intermediates)/main/marshal_generated.h: PRIVATE_XML := -f $(glapi)/gl_and_es_API.xml
> -
> -$(intermediates)/main/marshal_generated.h: $(dispatch_deps)
> - $(call es-gen)
> -
> GET_HASH_GEN := $(LOCAL_PATH)/main/get_hash_generator.py
>
> $(intermediates)/main/get_hash.h: PRIVATE_SCRIPT := $(MESA_PYTHON2) $(GET_HASH_GEN)
> diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
> index 86fbf39..e9b1b82 100644
> --- a/src/mesa/Makefile.sources
> +++ b/src/mesa/Makefile.sources
> @@ -137,7 +137,6 @@ MAIN_FILES = \
> main/marshal.c \
> main/marshal.h \
> main/marshal_generated.c \
> - main/marshal_generated.h \
> main/matrix.c \
> main/matrix.h \
> main/mipmap.c \
> diff --git a/src/mesa/SConscript b/src/mesa/SConscript
> index b63e15a..27bcb85 100644
> --- a/src/mesa/SConscript
> +++ b/src/mesa/SConscript
> @@ -133,14 +133,6 @@ env.CodeGenerate(
> command = python_cmd + ' $SCRIPT -f $SOURCE > $TARGET'
> )
>
> -# The marshal_generated.h file is generated from the GL/ES API.xml file
> -env.CodeGenerate(
> - target = 'main/marshal_generated.h',
> - script = GLAPI + 'gen/gl_marshal_h.py',
> - source = [GLAPI + 'gen/gl_and_es_API.xml'] + env.Glob(GLAPI + 'gen/*.xml'),
> - command = python_cmd + ' $SCRIPT -f $SOURCE > $TARGET'
> - )
> -
> #
> # Libraries
> #
> diff --git a/src/mesa/main/.gitignore b/src/mesa/main/.gitignore
> index 8cc33cf..31dae60 100644
> --- a/src/mesa/main/.gitignore
> +++ b/src/mesa/main/.gitignore
> @@ -10,4 +10,3 @@ format_info.c
> format_pack.c
> format_unpack.c
> marshal_generated.c
> -marshal_generated.h
> diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c
> index c71c037..8c7a8e0 100644
> --- a/src/mesa/main/glthread.c
> +++ b/src/mesa/main/glthread.c
> @@ -35,10 +35,15 @@
> #include "main/mtypes.h"
> #include "main/glthread.h"
> #include "main/marshal.h"
> -#include "main/marshal_generated.h"
> #include "util/u_atomic.h"
> #include "util/u_thread.h"
>
> +static size_t glthread_unmarshal_cmd(struct gl_context *ctx, const void *cmd)
> +{
> + const struct marshal_cmd_base *cmd_base = (struct marshal_cmd_base *)cmd;
> + cmd_base->cmd_func(ctx, cmd);
> + return cmd_base->cmd_size;
> +}
>
> static void
> glthread_unmarshal_batch(void *job, int thread_index)
> @@ -50,7 +55,7 @@ glthread_unmarshal_batch(void *job, int thread_index)
> _glapi_set_dispatch(ctx->CurrentServerDispatch);
>
> while (pos < batch->used)
> - pos += _mesa_unmarshal_dispatch_cmd(ctx, &batch->buffer[pos]);
> + pos += glthread_unmarshal_cmd(ctx, &batch->buffer[pos]);
>
> assert(pos == batch->used);
> batch->used = 0;
> diff --git a/src/mesa/main/glthread.h b/src/mesa/main/glthread.h
> index 306246c..07bccba 100644
> --- a/src/mesa/main/glthread.h
> +++ b/src/mesa/main/glthread.h
> @@ -49,8 +49,6 @@
> #include <stdbool.h>
> #include "util/u_queue.h"
>
> -enum marshal_dispatch_cmd_id;
> -
> /** A single batch of commands queued up for execution. */
> struct glthread_batch
> {
> diff --git a/src/mesa/main/marshal.c b/src/mesa/main/marshal.c
> index 8db4531..6c5c687 100644
> --- a/src/mesa/main/marshal.c
> +++ b/src/mesa/main/marshal.c
> @@ -31,7 +31,6 @@
> #include "main/macros.h"
> #include "marshal.h"
> #include "dispatch.h"
> -#include "marshal_generated.h"
>
> struct marshal_cmd_Flush
> {
> @@ -52,7 +51,7 @@ _mesa_marshal_Flush(void)
> {
> GET_CURRENT_CONTEXT(ctx);
> struct marshal_cmd_Flush *cmd =
> - _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_Flush,
> + _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_Flush,
> sizeof(struct marshal_cmd_Flush));
> (void) cmd;
> _mesa_post_marshal_hook(ctx);
> @@ -91,7 +90,7 @@ _mesa_marshal_Enable(GLenum cap)
> _mesa_glthread_finish(ctx);
> _mesa_glthread_restore_dispatch(ctx);
> } else {
> - cmd = _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_Enable,
> + cmd = _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_Enable,
> sizeof(*cmd));
> cmd->cap = cap;
> _mesa_post_marshal_hook(ctx);
> @@ -172,7 +171,7 @@ _mesa_marshal_ShaderSource(GLuint shader, GLsizei count,
>
> if (total_cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> struct marshal_cmd_ShaderSource *cmd =
> - _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_ShaderSource,
> + _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_ShaderSource,
> total_cmd_size);
> GLint *cmd_length = (GLint *) (cmd + 1);
> GLchar *cmd_strings = (GLchar *) (cmd_length + count);
> @@ -277,7 +276,7 @@ _mesa_marshal_BindBuffer(GLenum target, GLuint buffer)
> track_vbo_binding(ctx, target, buffer);
>
> if (cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> - cmd = _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_BindBuffer,
> + cmd = _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_BindBuffer,
> cmd_size);
> cmd->target = target;
> cmd->buffer = buffer;
> @@ -334,7 +333,7 @@ _mesa_marshal_BufferData(GLenum target, GLsizeiptr size, const GLvoid * data,
> if (target != GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD &&
> cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> struct marshal_cmd_BufferData *cmd =
> - _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_BufferData,
> + _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_BufferData,
> cmd_size);
>
> cmd->target = target;
> @@ -393,7 +392,7 @@ _mesa_marshal_BufferSubData(GLenum target, GLintptr offset, GLsizeiptr size,
> if (target != GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD &&
> cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> struct marshal_cmd_BufferSubData *cmd =
> - _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_BufferSubData,
> + _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_BufferSubData,
> cmd_size);
> cmd->target = target;
> cmd->offset = offset;
> @@ -447,7 +446,7 @@ _mesa_marshal_NamedBufferData(GLuint buffer, GLsizeiptr size,
>
> if (buffer > 0 && cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> struct marshal_cmd_NamedBufferData *cmd =
> - _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_NamedBufferData,
> + _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_NamedBufferData,
> cmd_size);
> cmd->name = buffer;
> cmd->size = size;
> @@ -501,7 +500,7 @@ _mesa_marshal_NamedBufferSubData(GLuint buffer, GLintptr offset,
>
> if (buffer > 0 && cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> struct marshal_cmd_NamedBufferSubData *cmd =
> - _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_NamedBufferSubData,
> + _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_NamedBufferSubData,
> cmd_size);
> cmd->name = buffer;
> cmd->offset = offset;
> @@ -569,7 +568,7 @@ _mesa_marshal_ClearBufferfv(GLenum buffer, GLint drawbuffer,
> size_t cmd_size = sizeof(struct marshal_cmd_ClearBufferfv) + size;
> if (cmd_size <= MARSHAL_MAX_CMD_SIZE) {
> struct marshal_cmd_ClearBufferfv *cmd =
> - _mesa_glthread_allocate_command(ctx, DISPATCH_CMD_ClearBufferfv,
> + _mesa_glthread_allocate_command(ctx, (unmarshal_func)_mesa_unmarshal_ClearBufferfv,
> cmd_size);
> cmd->buffer = buffer;
> cmd->drawbuffer = drawbuffer;
> diff --git a/src/mesa/main/marshal.h b/src/mesa/main/marshal.h
> index 999c75e..02893f8 100644
> --- a/src/mesa/main/marshal.h
> +++ b/src/mesa/main/marshal.h
> @@ -34,12 +34,15 @@
> #include "main/context.h"
> #include "main/macros.h"
>
> +
> +typedef void (*unmarshal_func)(struct gl_context *ctx, const void *data);
> +
> struct marshal_cmd_base
> {
> /**
> - * Type of command. See enum marshal_dispatch_cmd_id.
> + * Pointer to unmarshalling function.
> */
> - uint16_t cmd_id;
> + unmarshal_func cmd_func;
>
> /**
> * Size of command, in multiples of 4 bytes, including cmd_base.
> @@ -49,7 +52,7 @@ struct marshal_cmd_base
>
> static inline void *
> _mesa_glthread_allocate_command(struct gl_context *ctx,
> - uint16_t cmd_id,
> + unmarshal_func func,
> size_t size)
> {
> struct glthread_state *glthread = ctx->GLThread;
> @@ -64,7 +67,7 @@ _mesa_glthread_allocate_command(struct gl_context *ctx,
>
> cmd_base = (struct marshal_cmd_base *)&next->buffer[next->used];
> next->used += aligned_size;
> - cmd_base->cmd_id = cmd_id;
> + cmd_base->cmd_func = func;
> cmd_base->cmd_size = aligned_size;
> return cmd_base;
> }
> @@ -136,9 +139,6 @@ debug_print_unmarshal(const char *func)
> struct _glapi_table *
> _mesa_create_marshal_table(const struct gl_context *ctx);
>
> -size_t
> -_mesa_unmarshal_dispatch_cmd(struct gl_context *ctx, const void *cmd);
> -
> static inline void
> _mesa_post_marshal_hook(struct gl_context *ctx)
> {
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list