[Mesa-dev] [PATCH 2/4] mesa: Remove unnecessary locking from container objects.

Samuel Pitoiset samuel.pitoiset at gmail.com
Fri Apr 21 07:57:08 UTC 2017


Patches 1-3 are:

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

On 04/21/2017 07:20 AM, Timothy Arceri wrote:
> From: Matt Turner <mattst88 at gmail.com>
> 
>  From Chapter 5 'Shared Objects and Multiple Contexts' of
> the OpenGL 4.5 spec:
> 
>     "Objects which contain references to other objects include
>     framebuffer, program pipeline, query, transform feedback,
>     and vertex array objects.   Such objects are called container
>     objects and are not shared"
> 
> For we leave locking in place for framebuffer objects because
> the EXT fbo extension allowed sharing.
> 
> V2: (Timothy Arceri)
>   - rebased and dropped changes to framebuffer objects
> ---
>   src/mesa/main/arrayobj.c    | 6 ------
>   src/mesa/main/mtypes.h      | 4 ----
>   src/mesa/main/pipelineobj.c | 6 ------
>   src/mesa/main/shaderapi.c   | 3 ---
>   4 files changed, 19 deletions(-)
> 
> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
> index fdb3caa..9f4477e 100644
> --- a/src/mesa/main/arrayobj.c
> +++ b/src/mesa/main/arrayobj.c
> @@ -162,21 +162,20 @@ _mesa_new_vao(struct gl_context *ctx, GLuint name)
>   
>   
>   /**
>    * Delete an array object.
>    */
>   void
>   _mesa_delete_vao(struct gl_context *ctx, struct gl_vertex_array_object *obj)
>   {
>      unbind_array_object_vbos(ctx, obj);
>      _mesa_reference_buffer_object(ctx, &obj->IndexBufferObj, NULL);
> -   mtx_destroy(&obj->Mutex);
>      free(obj->Label);
>      free(obj);
>   }
>   
>   
>   /**
>    * Set ptr to vao w/ reference counting.
>    * Note: this should only be called from the _mesa_reference_vao()
>    * inline function.
>    */
> @@ -185,41 +184,37 @@ _mesa_reference_vao_(struct gl_context *ctx,
>                        struct gl_vertex_array_object **ptr,
>                        struct gl_vertex_array_object *vao)
>   {
>      assert(*ptr != vao);
>   
>      if (*ptr) {
>         /* Unreference the old array object */
>         GLboolean deleteFlag = GL_FALSE;
>         struct gl_vertex_array_object *oldObj = *ptr;
>   
> -      mtx_lock(&oldObj->Mutex);
>         assert(oldObj->RefCount > 0);
>         oldObj->RefCount--;
>         deleteFlag = (oldObj->RefCount == 0);
> -      mtx_unlock(&oldObj->Mutex);
>   
>         if (deleteFlag)
>            _mesa_delete_vao(ctx, oldObj);
>   
>         *ptr = NULL;
>      }
>      assert(!*ptr);
>   
>      if (vao) {
>         /* reference new array object */
> -      mtx_lock(&vao->Mutex);
>         assert(vao->RefCount > 0);
>   
>         vao->RefCount++;
>         *ptr = vao;
> -      mtx_unlock(&vao->Mutex);
>      }
>   }
>   
>   
>   /**
>    * Initialize attribtes of a vertex array within a vertex array object.
>    * \param vao  the container vertex array object
>    * \param index  which array in the VAO to initialize
>    * \param size  number of components (1, 2, 3 or 4) per attribute
>    * \param type  datatype of the attribute (GL_FLOAT, GL_INT, etc).
> @@ -261,21 +256,20 @@ init_array(struct gl_context *ctx,
>    */
>   void
>   _mesa_initialize_vao(struct gl_context *ctx,
>                        struct gl_vertex_array_object *vao,
>                        GLuint name)
>   {
>      GLuint i;
>   
>      vao->Name = name;
>   
> -   mtx_init(&vao->Mutex, mtx_plain);
>      vao->RefCount = 1;
>   
>      /* Init the individual arrays */
>      for (i = 0; i < ARRAY_SIZE(vao->VertexAttrib); i++) {
>         switch (i) {
>         case VERT_ATTRIB_WEIGHT:
>            init_array(ctx, vao, VERT_ATTRIB_WEIGHT, 1, GL_FLOAT);
>            break;
>         case VERT_ATTRIB_NORMAL:
>            init_array(ctx, vao, VERT_ATTRIB_NORMAL, 3, GL_FLOAT);
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index c4fab9d..b3711ba 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1502,22 +1502,20 @@ struct gl_vertex_buffer_binding
>    */
>   struct gl_vertex_array_object
>   {
>      /** Name of the VAO as received from glGenVertexArray. */
>      GLuint Name;
>   
>      GLint RefCount;
>   
>      GLchar *Label;       /**< GL_KHR_debug */
>   
> -   mtx_t Mutex;
> -
>      /**
>       * Does the VAO use ARB semantics or Apple semantics?
>       *
>       * There are several ways in which ARB_vertex_array_object and
>       * APPLE_vertex_array_object VAOs have differing semantics.  At the very
>       * least,
>       *
>       *     - ARB VAOs require that all array data be sourced from vertex buffer
>       *       objects, but Apple VAOs do not.
>       *
> @@ -2994,22 +2992,20 @@ struct gl_shader_program
>    */
>   struct gl_pipeline_object
>   {
>      /** Name of the pipeline object as received from glGenProgramPipelines.
>       * It would be 0 for shaders without separate shader objects.
>       */
>      GLuint Name;
>   
>      GLint RefCount;
>   
> -   mtx_t Mutex;
> -
>      GLchar *Label;   /**< GL_KHR_debug */
>   
>      /**
>       * Programs used for rendering
>       *
>       * There is a separate program set for each shader stage.
>       */
>      struct gl_program *CurrentProgram[MESA_SHADER_STAGES];
>   
>      struct gl_shader_program *ReferencedPrograms[MESA_SHADER_STAGES];
> diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
> index 2988c97..a0fa55a 100644
> --- a/src/mesa/main/pipelineobj.c
> +++ b/src/mesa/main/pipelineobj.c
> @@ -59,35 +59,33 @@ _mesa_delete_pipeline_object(struct gl_context *ctx,
>      unsigned i;
>   
>      _mesa_reference_program(ctx, &obj->_CurrentFragmentProgram, NULL);
>   
>      for (i = 0; i < MESA_SHADER_STAGES; i++) {
>         _mesa_reference_program(ctx, &obj->CurrentProgram[i], NULL);
>         _mesa_reference_shader_program(ctx, &obj->ReferencedPrograms[i], NULL);
>      }
>   
>      _mesa_reference_shader_program(ctx, &obj->ActiveProgram, NULL);
> -   mtx_destroy(&obj->Mutex);
>      free(obj->Label);
>      ralloc_free(obj);
>   }
>   
>   /**
>    * Allocate and initialize a new pipeline object.
>    */
>   static struct gl_pipeline_object *
>   _mesa_new_pipeline_object(struct gl_context *ctx, GLuint name)
>   {
>      struct gl_pipeline_object *obj = rzalloc(NULL, struct gl_pipeline_object);
>      if (obj) {
>         obj->Name = name;
> -      mtx_init(&obj->Mutex, mtx_plain);
>         obj->RefCount = 1;
>         obj->Flags = _mesa_get_shader_flags();
>         obj->InfoLog = NULL;
>      }
>   
>      return obj;
>   }
>   
>   /**
>    * Initialize pipeline object state for given context.
> @@ -182,42 +180,38 @@ _mesa_reference_pipeline_object_(struct gl_context *ctx,
>                                    struct gl_pipeline_object **ptr,
>                                    struct gl_pipeline_object *obj)
>   {
>      assert(*ptr != obj);
>   
>      if (*ptr) {
>         /* Unreference the old pipeline object */
>         GLboolean deleteFlag = GL_FALSE;
>         struct gl_pipeline_object *oldObj = *ptr;
>   
> -      mtx_lock(&oldObj->Mutex);
>         assert(oldObj->RefCount > 0);
>         oldObj->RefCount--;
>         deleteFlag = (oldObj->RefCount == 0);
> -      mtx_unlock(&oldObj->Mutex);
>   
>         if (deleteFlag) {
>            _mesa_delete_pipeline_object(ctx, oldObj);
>         }
>   
>         *ptr = NULL;
>      }
>      assert(!*ptr);
>   
>      if (obj) {
>         /* reference new pipeline object */
> -      mtx_lock(&obj->Mutex);
>         assert(obj->RefCount > 0);
>   
>         obj->RefCount++;
>         *ptr = obj;
> -      mtx_unlock(&obj->Mutex);
>      }
>   }
>   
>   static void
>   use_program_stage(struct gl_context *ctx, GLenum type,
>                     struct gl_shader_program *shProg,
>                     struct gl_pipeline_object *pipe) {
>      gl_shader_stage stage = _mesa_shader_enum_to_shader_stage(type);
>      struct gl_program *prog = NULL;
>      if (shProg && shProg->_LinkedShaders[stage])
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 187475f..c41f006 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -131,22 +131,20 @@ _mesa_init_shader_state(struct gl_context *ctx)
>      for (sh = 0; sh < MESA_SHADER_STAGES; ++sh)
>         memcpy(&ctx->Const.ShaderCompilerOptions[sh], &options, sizeof(options));
>   
>      ctx->Shader.Flags = _mesa_get_shader_flags();
>   
>      if (ctx->Shader.Flags != 0)
>         ctx->Const.GenerateTemporaryNames = true;
>   
>      /* Extended for ARB_separate_shader_objects */
>      ctx->Shader.RefCount = 1;
> -   mtx_init(&ctx->Shader.Mutex, mtx_plain);
> -
>      ctx->TessCtrlProgram.patch_vertices = 3;
>      for (i = 0; i < 4; ++i)
>         ctx->TessCtrlProgram.patch_default_outer_level[i] = 1.0;
>      for (i = 0; i < 2; ++i)
>         ctx->TessCtrlProgram.patch_default_inner_level[i] = 1.0;
>   }
>   
>   
>   /**
>    * Free the per-context shader-related state.
> @@ -157,21 +155,20 @@ _mesa_free_shader_state(struct gl_context *ctx)
>      for (int i = 0; i < MESA_SHADER_STAGES; i++) {
>         _mesa_reference_program(ctx, &ctx->Shader.CurrentProgram[i], NULL);
>      }
>      _mesa_reference_program(ctx, &ctx->Shader._CurrentFragmentProgram, NULL);
>      _mesa_reference_shader_program(ctx, &ctx->Shader.ActiveProgram, NULL);
>   
>      /* Extended for ARB_separate_shader_objects */
>      _mesa_reference_pipeline_object(ctx, &ctx->_Shader, NULL);
>   
>      assert(ctx->Shader.RefCount == 1);
> -   mtx_destroy(&ctx->Shader.Mutex);
>   }
>   
>   
>   /**
>    * Copy string from <src> to <dst>, up to maxLength characters, returning
>    * length of <dst> in <length>.
>    * \param src  the strings source
>    * \param maxLength  max chars to copy
>    * \param length  returns number of chars copied
>    * \param dst  the string destination
> 


More information about the mesa-dev mailing list