[Mesa-dev] [PATCH 4/4] mesa: don't lock hashtables that are not shared across contexts
Samuel Pitoiset
samuel.pitoiset at gmail.com
Fri Apr 21 07:50:24 UTC 2017
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);
> }
> }
> }
>
More information about the mesa-dev
mailing list