From 8e7270de551e25acc75b2acb496320cf312c96ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Sat, 3 Oct 2020 16:52:25 -0400 Subject: [PATCH] mesa: lock Shared->BufferObjects only once for a glthread batch This removes a lot of locking from the driver thread. If multiple contexts sharing buffers submit GL calls from multiple threads, they will be serialized by this mutex. I can add a driconf option to turn off this optimization if needed, but I currently don't anticipate to see GL apps that use multiple shared contexts in different threads simultaneously. Acked-by: Pierre-Eric Pelloux-Prayer Part-of: --- src/mesa/main/bufferobj.c | 49 ++++++++++++++++++++++++++------------- src/mesa/main/glthread.c | 7 ++++++ src/mesa/main/hash.h | 45 +++++++++++++++++++++++++++++++++++ src/mesa/main/mtypes.h | 3 +++ src/mesa/main/varray.c | 6 +++-- 5 files changed, 92 insertions(+), 18 deletions(-) diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index 08a3a550c22..f667ece819b 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -590,7 +590,8 @@ _mesa_total_buffer_object_memory(struct gl_context *ctx) { GLuint total = 0; - _mesa_HashWalk(ctx->Shared->BufferObjects, count_buffer_size, &total); + _mesa_HashWalkMaybeLocked(ctx->Shared->BufferObjects, count_buffer_size, + &total, ctx->BufferObjectsLocked); return total; } @@ -969,7 +970,9 @@ _mesa_handle_bind_buffer_gen(struct gl_context *ctx, _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", caller); return false; } - _mesa_HashInsert(ctx->Shared->BufferObjects, buffer, *buf_handle, buf != NULL); + _mesa_HashInsertMaybeLocked(ctx->Shared->BufferObjects, buffer, + *buf_handle, buf != NULL, + ctx->BufferObjectsLocked); } return true; @@ -1042,7 +1045,8 @@ _mesa_lookup_bufferobj(struct gl_context *ctx, GLuint buffer) return NULL; else return (struct gl_buffer_object *) - _mesa_HashLookup(ctx->Shared->BufferObjects, buffer); + _mesa_HashLookupMaybeLocked(ctx->Shared->BufferObjects, buffer, + ctx->BufferObjectsLocked); } @@ -1454,7 +1458,8 @@ delete_buffers(struct gl_context *ctx, GLsizei n, const GLuint *ids) { FLUSH_VERTICES(ctx, 0); - _mesa_HashLockMutex(ctx->Shared->BufferObjects); + _mesa_HashLockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); for (GLsizei i = 0; i < n; i++) { struct gl_buffer_object *bufObj = @@ -1585,7 +1590,8 @@ delete_buffers(struct gl_context *ctx, GLsizei n, const GLuint *ids) } } - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); } @@ -1627,7 +1633,8 @@ create_buffers(struct gl_context *ctx, GLsizei n, GLuint *buffers, bool dsa) /* * This must be atomic (generation and allocation of buffer object IDs) */ - _mesa_HashLockMutex(ctx->Shared->BufferObjects); + _mesa_HashLockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); _mesa_HashFindFreeKeys(ctx->Shared->BufferObjects, buffers, n); @@ -1641,7 +1648,8 @@ create_buffers(struct gl_context *ctx, GLsizei n, GLuint *buffers, bool dsa) buf = ctx->Driver.NewBufferObject(ctx, buffers[i]); if (!buf) { _mesa_error(ctx, GL_OUT_OF_MEMORY, "glCreateBuffers"); - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); return; } } @@ -1651,7 +1659,8 @@ create_buffers(struct gl_context *ctx, GLsizei n, GLuint *buffers, bool dsa) _mesa_HashInsertLocked(ctx->Shared->BufferObjects, buffers[i], buf, true); } - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); } @@ -4058,7 +4067,8 @@ bind_uniform_buffers(struct gl_context *ctx, GLuint first, GLsizei count, * parameters are valid and no other error occurs." */ - _mesa_HashLockMutex(ctx->Shared->BufferObjects); + _mesa_HashLockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); for (int i = 0; i < count; i++) { struct gl_buffer_binding *binding = @@ -4109,7 +4119,8 @@ bind_uniform_buffers(struct gl_context *ctx, GLuint first, GLsizei count, USAGE_UNIFORM_BUFFER); } - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); } static void @@ -4159,7 +4170,8 @@ bind_shader_storage_buffers(struct gl_context *ctx, GLuint first, * parameters are valid and no other error occurs." */ - _mesa_HashLockMutex(ctx->Shared->BufferObjects); + _mesa_HashLockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); for (int i = 0; i < count; i++) { struct gl_buffer_binding *binding = @@ -4210,7 +4222,8 @@ bind_shader_storage_buffers(struct gl_context *ctx, GLuint first, USAGE_SHADER_STORAGE_BUFFER); } - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); } static bool @@ -4325,7 +4338,8 @@ bind_xfb_buffers(struct gl_context *ctx, * parameters are valid and no other error occurs." */ - _mesa_HashLockMutex(ctx->Shared->BufferObjects); + _mesa_HashLockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); for (int i = 0; i < count; i++) { const GLuint index = first + i; @@ -4392,7 +4406,8 @@ bind_xfb_buffers(struct gl_context *ctx, offset, size); } - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); } static bool @@ -4484,7 +4499,8 @@ bind_atomic_buffers(struct gl_context *ctx, * parameters are valid and no other error occurs." */ - _mesa_HashLockMutex(ctx->Shared->BufferObjects); + _mesa_HashLockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); for (int i = 0; i < count; i++) { struct gl_buffer_binding *binding = @@ -4532,7 +4548,8 @@ bind_atomic_buffers(struct gl_context *ctx, USAGE_ATOMIC_COUNTER_BUFFER); } - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); } static ALWAYS_INLINE void diff --git a/src/mesa/main/glthread.c b/src/mesa/main/glthread.c index 53d9f03a252..cf50ce45496 100644 --- a/src/mesa/main/glthread.c +++ b/src/mesa/main/glthread.c @@ -52,6 +52,9 @@ glthread_unmarshal_batch(void *job, int thread_index) _glapi_set_dispatch(ctx->CurrentServerDispatch); + _mesa_HashLockMutex(ctx->Shared->BufferObjects); + ctx->BufferObjectsLocked = true; + while (pos < used) { const struct marshal_cmd_base *cmd = (const struct marshal_cmd_base *)&buffer[pos]; @@ -60,12 +63,16 @@ glthread_unmarshal_batch(void *job, int thread_index) pos += cmd->cmd_size; } + ctx->BufferObjectsLocked = false; + _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + assert(pos == used); batch->used = 0; unsigned batch_index = batch - ctx->GLThread.batches; /* Atomically set this to -1 if it's equal to batch_index. */ p_atomic_cmpxchg(&ctx->GLThread.LastProgramChangeBatch, batch_index, -1); + } static void diff --git a/src/mesa/main/hash.h b/src/mesa/main/hash.h index 03eadd6ab8f..7aa1e660917 100644 --- a/src/mesa/main/hash.h +++ b/src/mesa/main/hash.h @@ -188,5 +188,50 @@ extern void _mesa_test_hash_functions(void); extern void _mesa_HashEnableNameReuse(struct _mesa_HashTable *table); +static inline void +_mesa_HashWalkMaybeLocked(const struct _mesa_HashTable *table, + void (*callback)(void *data, void *userData), + void *userData, bool locked) +{ + if (locked) + _mesa_HashWalkLocked(table, callback, userData); + else + _mesa_HashWalk(table, callback, userData); +} + +static inline struct gl_buffer_object * +_mesa_HashLookupMaybeLocked(struct _mesa_HashTable *table, GLuint key, + bool locked) +{ + if (locked) + return _mesa_HashLookupLocked(table, key); + else + return _mesa_HashLookup(table, key); +} + +static inline void +_mesa_HashInsertMaybeLocked(struct _mesa_HashTable *table, + GLuint key, void *data, GLboolean isGenName, + bool locked) +{ + if (locked) + _mesa_HashInsertLocked(table, key, data, isGenName); + else + _mesa_HashInsert(table, key, data, isGenName); +} + +static inline void +_mesa_HashLockMaybeLocked(struct _mesa_HashTable *table, bool locked) +{ + if (!locked) + _mesa_HashLockMutex(table); +} + +static inline void +_mesa_HashUnlockMaybeLocked(struct _mesa_HashTable *table, bool locked) +{ + if (!locked) + _mesa_HashUnlockMutex(table); +} #endif diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index bd0ae969e94..8302416d50a 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -4926,6 +4926,9 @@ struct gl_context /** State possibly shared with other contexts in the address space */ struct gl_shared_state *Shared; + /** Whether Shared->BufferObjects has already been locked for this context. */ + bool BufferObjectsLocked; + /** \name API function pointer tables */ /*@{*/ gl_api API; diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c index ec5d09ef151..677dd967979 100644 --- a/src/mesa/main/varray.c +++ b/src/mesa/main/varray.c @@ -3130,7 +3130,8 @@ vertex_array_vertex_buffers(struct gl_context *ctx, * their parameters are valid and no other error occurs." */ - _mesa_HashLockMutex(ctx->Shared->BufferObjects); + _mesa_HashLockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); for (i = 0; i < count; i++) { struct gl_buffer_object *vbo; @@ -3187,7 +3188,8 @@ vertex_array_vertex_buffers(struct gl_context *ctx, vbo, offsets[i], strides[i], false, false); } - _mesa_HashUnlockMutex(ctx->Shared->BufferObjects); + _mesa_HashUnlockMaybeLocked(ctx->Shared->BufferObjects, + ctx->BufferObjectsLocked); }