[Mesa-dev] [PATCH 4/4] mesa: don't lock hashtables that are not shared across contexts
Timothy Arceri
tarceri at itsqueeze.com
Fri Apr 21 09:47:24 UTC 2017
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.
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