[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 09:48:32 UTC 2017
On 04/21/2017 11:47 AM, Timothy Arceri wrote:
> On 21/04/17 17:50, Samuel Pitoiset wrote:
>> 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?
>
> This hash table does some additional things that the util hash doesn't
> provide. For now I'd rather just push this.
Fine by me.
Feel free to add my Rb on this one as well.
>
> We really need a newbie/easy task list to add stuff like 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