I would definitely prefer to see _mesa_hash_table instead here. That way you can get rid of the unnecessary locking at the same time.

I actually use _mesa_hash_table for the resident texture/image handles as part of ARB_bindless_texture. Though, I use the locked hash_table implementation inside the shared context, of course.

Anyways, we shouldn't use any locked hash tables in the per-context object.

Can you change this?

On 04/21/2017 07:20 AM, Timothy Arceri wrote:
 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.

We could maybe just replace the hash with an ordinary hash table
but for now this should remove most of the unnecessary locking.
---
  src/mesa/main/arrayobj.c          | 8 ++++----
  src/mesa/main/pipelineobj.c       | 6 +++---
  src/mesa/main/queryobj.c          | 8 ++++----
  src/mesa/main/queryobj.h          | 2 +-
  src/mesa/main/transformfeedback.c | 7 ++++---
  5 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index 24555d9..b901a89 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -64,21 +64,21 @@
   * non-existent.
   */
struct gl_vertex_array_object *
  _mesa_lookup_vao(struct gl_context *ctx, GLuint id)
  {
     if (id == 0)
        return NULL;
     else
        return (struct gl_vertex_array_object *)
-         _mesa_HashLookup(ctx->Array.Objects, id);
+         _mesa_HashLookupLocked(ctx->Array.Objects, id);
  }
/**
   * Looks up the array object for the given ID.
   *
   * Unlike _mesa_lookup_vao, this function generates a GL_INVALID_OPERATION
   * error if the array object does not exist. It also returns the default
   * array object when ctx is a compatibility profile context and id is zero.
   */
@@ -101,21 +101,21 @@ _mesa_lookup_vao_err(struct gl_context *ctx, GLuint id, 
const char *caller)
return ctx->Array.DefaultVAO;
     } else {
        struct gl_vertex_array_object *vao;
if (ctx->Array.LastLookedUpVAO &&
            ctx->Array.LastLookedUpVAO->Name == id) {
           vao = ctx->Array.LastLookedUpVAO;
        } else {
           vao = (struct gl_vertex_array_object *)
-            _mesa_HashLookup(ctx->Array.Objects, id);
+            _mesa_HashLookupLocked(ctx->Array.Objects, id);
/* The ARB_direct_state_access specification says:
            *
            *    "An INVALID_OPERATION error is generated if <vaobj> is not
            *     [compatibility profile: zero or] the name of an existing
            *     vertex array object."
            */
           if (!vao || !vao->EverBound) {
              _mesa_error(ctx, GL_INVALID_OPERATION,
                          "%s(non-existent vaobj=%u)", caller, id);
@@ -299,35 +299,35 @@ _mesa_initialize_vao(struct gl_context *ctx,
/**
   * Add the given array object to the array object pool.
   */
  static void
  save_array_object(struct gl_context *ctx, struct gl_vertex_array_object *vao)
  {
     if (vao->Name > 0) {
        /* insert into hash table */
-      _mesa_HashInsert(ctx->Array.Objects, vao->Name, vao);
+      _mesa_HashInsertLocked(ctx->Array.Objects, vao->Name, vao);
     }
  }
/**
   * Remove the given array object from the array object pool.
   * Do not deallocate the array object though.
   */
  static void
  remove_array_object(struct gl_context *ctx, struct gl_vertex_array_object 
*vao)
  {
     if (vao->Name > 0) {
        /* remove from hash table */
-      _mesa_HashRemove(ctx->Array.Objects, vao->Name);
+      _mesa_HashRemoveLocked(ctx->Array.Objects, vao->Name);
     }
  }
/**
   * Updates the derived gl_vertex_arrays when a gl_vertex_attrib_array
   * or a gl_vertex_buffer_binding has changed.
   */
  void
  _mesa_update_vao_client_arrays(struct gl_context *ctx,
diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index 9a852be..dbca3c3 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -137,43 +137,43 @@ _mesa_free_pipeline_data(struct gl_context *ctx)
   * a non-existent ID.  The spec defines ID 0 as being technically
   * non-existent.
   */
  struct gl_pipeline_object *
  _mesa_lookup_pipeline_object(struct gl_context *ctx, GLuint id)
  {
     if (id == 0)
        return NULL;
     else
        return (struct gl_pipeline_object *)
-         _mesa_HashLookup(ctx->Pipeline.Objects, id);
+         _mesa_HashLookupLocked(ctx->Pipeline.Objects, id);
  }
/**
   * Add the given pipeline object to the pipeline object pool.
   */
  static void
  save_pipeline_object(struct gl_context *ctx, struct gl_pipeline_object *obj)
  {
     if (obj->Name > 0) {
-      _mesa_HashInsert(ctx->Pipeline.Objects, obj->Name, obj);
+      _mesa_HashInsertLocked(ctx->Pipeline.Objects, obj->Name, obj);
     }
  }
/**
   * Remove the given pipeline object from the pipeline object pool.
   * Do not deallocate the pipeline object though.
   */
  static void
  remove_pipeline_object(struct gl_context *ctx, struct gl_pipeline_object *obj)
  {
     if (obj->Name > 0) {
-      _mesa_HashRemove(ctx->Pipeline.Objects, obj->Name);
+      _mesa_HashRemoveLocked(ctx->Pipeline.Objects, obj->Name);
     }
  }
/**
   * Set ptr to obj w/ reference counting.
   * Note: this should only be called from the _mesa_reference_pipeline_object()
   * inline function.
   */
  void
  _mesa_reference_pipeline_object_(struct gl_context *ctx,
diff --git a/src/mesa/main/queryobj.c b/src/mesa/main/queryobj.c
index e4edb51..46535d7 100644
--- a/src/mesa/main/queryobj.c
+++ b/src/mesa/main/queryobj.c
@@ -271,21 +271,21 @@ create_queries(struct gl_context *ctx, GLenum target, 
GLsizei n, GLuint *ids,
              = ctx->Driver.NewQueryObject(ctx, first + i);
           if (!q) {
              _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
              return;
           } else if (dsa) {
              /* Do the equivalent of binding the buffer with a target */
              q->Target = target;
              q->EverBound = GL_TRUE;
           }
           ids[i] = first + i;
-         _mesa_HashInsert(ctx->Query.QueryObjects, first + i, q);
+         _mesa_HashInsertLocked(ctx->Query.QueryObjects, first + i, q);
        }
     }
  }
void GLAPIENTRY
  _mesa_GenQueries(GLsizei n, GLuint *ids)
  {
     GET_CURRENT_CONTEXT(ctx);
     create_queries(ctx, 0, n, ids, false);
  }
@@ -338,21 +338,21 @@ _mesa_DeleteQueries(GLsizei n, const GLuint *ids)
              if (q->Active) {
                 struct gl_query_object **bindpt;
                 bindpt = get_query_binding_point(ctx, q->Target, q->Stream);
                 assert(bindpt); /* Should be non-null for active q. */
                 if (bindpt) {
                    *bindpt = NULL;
                 }
                 q->Active = GL_FALSE;
                 ctx->Driver.EndQuery(ctx, q);
              }
-            _mesa_HashRemove(ctx->Query.QueryObjects, ids[i]);
+            _mesa_HashRemoveLocked(ctx->Query.QueryObjects, ids[i]);
              ctx->Driver.DeleteQuery(ctx, q);
           }
        }
     }
  }
GLboolean GLAPIENTRY
  _mesa_IsQuery(GLuint id)
  {
@@ -441,21 +441,21 @@ _mesa_BeginQueryIndexed(GLenum target, GLuint index, 
GLuint id)
           _mesa_error(ctx, GL_INVALID_OPERATION,
                       "glBeginQuery{Indexed}(non-gen name)");
           return;
        } else {
           /* create new object */
           q = ctx->Driver.NewQueryObject(ctx, id);
           if (!q) {
              _mesa_error(ctx, GL_OUT_OF_MEMORY, "glBeginQuery{Indexed}");
              return;
           }
-         _mesa_HashInsert(ctx->Query.QueryObjects, id, q);
+         _mesa_HashInsertLocked(ctx->Query.QueryObjects, id, q);
        }
     }
     else {
        /* pre-existing object */
        if (q->Active) {
           _mesa_error(ctx, GL_INVALID_OPERATION,
                       "glBeginQuery{Indexed}(query already active)");
           return;
        }
@@ -583,21 +583,21 @@ _mesa_QueryCounter(GLuint id, GLenum target)
     q = _mesa_lookup_query_object(ctx, id);
     if (!q) {
        /* XXX the Core profile should throw INVALID_OPERATION here */
/* create new object */
        q = ctx->Driver.NewQueryObject(ctx, id);
        if (!q) {
           _mesa_error(ctx, GL_OUT_OF_MEMORY, "glQueryCounter");
           return;
        }
-      _mesa_HashInsert(ctx->Query.QueryObjects, id, q);
+      _mesa_HashInsertLocked(ctx->Query.QueryObjects, id, q);
     }
     else {
        if (q->Target && q->Target != GL_TIMESTAMP) {
           _mesa_error(ctx, GL_INVALID_OPERATION,
                       "glQueryCounter(id has an invalid target)");
           return;
        }
     }
if (q->Active) {
diff --git a/src/mesa/main/queryobj.h b/src/mesa/main/queryobj.h
index d1036fc..24a8257 100644
--- a/src/mesa/main/queryobj.h
+++ b/src/mesa/main/queryobj.h
@@ -28,21 +28,21 @@
#include "main/mtypes.h"
  #include "main/hash.h"
static inline struct gl_query_object *
  _mesa_lookup_query_object(struct gl_context *ctx, GLuint id)
  {
     return (struct gl_query_object *)
-      _mesa_HashLookup(ctx->Query.QueryObjects, id);
+      _mesa_HashLookupLocked(ctx->Query.QueryObjects, id);
  }
extern void
  _mesa_init_query_object_functions(struct dd_function_table *driver);
extern void
  _mesa_init_queryobj(struct gl_context *ctx);
extern void
diff --git a/src/mesa/main/transformfeedback.c 
b/src/mesa/main/transformfeedback.c
index 131014f..3c72674 100644
--- a/src/mesa/main/transformfeedback.c
+++ b/src/mesa/main/transformfeedback.c
@@ -958,21 +958,21 @@ _mesa_lookup_transform_feedback_object(struct gl_context 
*ctx, GLuint name)
  {
     /* OpenGL 4.5 core profile, 13.2 pdf page 444: "xfb must be zero, 
indicating
      * the default transform feedback object, or the name of an existing
      * transform feedback object."
      */
     if (name == 0) {
        return ctx->TransformFeedback.DefaultObject;
     }
     else
        return (struct gl_transform_feedback_object *)
-         _mesa_HashLookup(ctx->TransformFeedback.Objects, name);
+         _mesa_HashLookupLocked(ctx->TransformFeedback.Objects, name);
  }
static void
  create_transform_feedbacks(struct gl_context *ctx, GLsizei n, GLuint *ids,
                             bool dsa)
  {
     GLuint first;
     const char* func;
if (dsa)
@@ -993,21 +993,22 @@ create_transform_feedbacks(struct gl_context *ctx, 
GLsizei n, GLuint *ids,
     if (first) {
        GLsizei i;
        for (i = 0; i < n; i++) {
           struct gl_transform_feedback_object *obj
              = ctx->Driver.NewTransformFeedback(ctx, first + i);
           if (!obj) {
              _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
              return;
           }
           ids[i] = first + i;
-         _mesa_HashInsert(ctx->TransformFeedback.Objects, first + i, obj);
+         _mesa_HashInsertLocked(ctx->TransformFeedback.Objects, first + i,
+                                obj);
           if (dsa) {
              /* this is normally done at bind time in the non-dsa case */
              obj->EverBound = GL_TRUE;
           }
        }
     }
     else {
        _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", func);
     }
  }
@@ -1125,21 +1126,21 @@ _mesa_DeleteTransformFeedbacks(GLsizei n, const GLuint 
*names)
        if (names[i] > 0) {
           struct gl_transform_feedback_object *obj
              = _mesa_lookup_transform_feedback_object(ctx, names[i]);
           if (obj) {
              if (obj->Active) {
                 _mesa_error(ctx, GL_INVALID_OPERATION,
                             "glDeleteTransformFeedbacks(object %u is active)",
                             names[i]);
                 return;
              }
-            _mesa_HashRemove(ctx->TransformFeedback.Objects, names[i]);
+            _mesa_HashRemoveLocked(ctx->TransformFeedback.Objects, names[i]);
              /* unref, but object may not be deleted until later */
              if (obj == ctx->TransformFeedback.CurrentObject) {
                 reference_transform_feedback_object(
                       &ctx->TransformFeedback.CurrentObject,
                       ctx->TransformFeedback.DefaultObject);
              }
              reference_transform_feedback_object(&obj, NULL);
           }
        }
     }

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to