[Mesa-dev] [RFC 2/2] glx: drop GLX_MESA_multithread_makecurrent support
Samuel Pitoiset
samuel.pitoiset at gmail.com
Fri Apr 21 07:45:17 UTC 2017
+1 on the idea as well, that would help in order to remove some
unnecessary locking, although not in a critical path.
On 04/19/2017 09:45 AM, Timothy Arceri wrote:
> This extension is not supported by GLVND, also as far
> as I can tell this extension requires us to do extra
> locking for objects that are not normaly shared across
> contexts, like vertex array and pipeline objects.
>
> Glthread also won't play nice with this extension.
>
> Finally it seem it is no longer used by gstreamer.
> ---
>
> I wasn't sure if the spec should be deleted or if we should keep it
> in docs anyway. I deleted it for now.
>
> docs/relnotes/17.2.0.html | 2 +-
> docs/specs/MESA_multithread_makecurrent.spec | 158 ---------------------------
> src/glx/glxclient.h | 4 +-
> src/glx/glxcurrent.c | 34 +++---
> src/glx/glxextensions.c | 2 +-
> src/mapi/mapi_glapi.c | 6 +-
> 6 files changed, 21 insertions(+), 185 deletions(-)
> delete mode 100644 docs/specs/MESA_multithread_makecurrent.spec
>
> diff --git a/docs/relnotes/17.2.0.html b/docs/relnotes/17.2.0.html
> index 5d9d7db..ef3d473 100644
> --- a/docs/relnotes/17.2.0.html
> +++ b/docs/relnotes/17.2.0.html
> @@ -49,16 +49,16 @@ TBD
>
> <h2>Bug fixes</h2>
>
> <ul>
> TBD
> </ul>
>
> <h2>Changes</h2>
>
> <ul>
> -TBD
> +<li>Removed GLX_MESA_multithread_makecurrent support.</li>
> </ul>
>
> </div>
> </body>
> </html>
> diff --git a/docs/specs/MESA_multithread_makecurrent.spec b/docs/specs/MESA_multithread_makecurrent.spec
> deleted file mode 100644
> index 5065c2f..0000000
> --- a/docs/specs/MESA_multithread_makecurrent.spec
> +++ /dev/null
> @@ -1,158 +0,0 @@
> -Name
> -
> - MESA_multithread_makecurrent
> -
> -Name Strings
> -
> - GLX_MESA_multithread_makecurrent
> -
> -Contact
> -
> - Eric Anholt (eric at anholt.net)
> -
> -Status
> -
> - Not shipping.
> -
> -Version
> -
> - Last Modified Date: 21 February 2011
> -
> -Number
> -
> - TBD
> -
> -Dependencies
> -
> - OpenGL 1.0 or later is required.
> - GLX 1.3 or later is required.
> -
> -Overview
> -
> - The GLX context setup encourages multithreaded applications to
> - create a context per thread which each operate on their own
> - objects in parallel, and leaves synchronization for write access
> - to shared objects up to the application.
> -
> - For some applications, maintaining per-thread contexts and
> - ensuring that the glFlush happens in one thread before another
> - thread starts working on that object is difficult. For them,
> - using the same context across multiple threads and protecting its
> - usage with a mutex is both higher performance and easier to
> - implement. This extension gives those applications that option by
> - relaxing the context binding requirements.
> -
> - This new behavior matches the requirements of AGL, while providing
> - a feature not specified in WGL.
> -
> -IP Status
> -
> - Open-source; freely implementable.
> -
> -Issues
> -
> - None.
> -
> -New Procedures and Functions
> -
> - None.
> -
> -New Tokens
> -
> - None.
> -
> -Changes to Chapter 2 of the GLX 1.3 Specification (Functions and Errors)
> -
> - Replace the following sentence from section 2.2 Rendering Contexts:
> - In addition, a rendering context can be current for only one
> - thread at a time.
> - with:
> - In addition, an indirect rendering context can be current for
> - only one thread at a time. A direct rendering context may be
> - current to multiple threads, with synchronization of access to
> - the context thruogh the GL managed by the application through
> - mutexes.
> -
> -Changes to Chapter 3 of the GLX 1.3 Specification (Functions and Errors)
> -
> - Replace the following sentence from section 3.3.7 Rendering Contexts:
> - If ctx is current to some other thread, then
> - glXMakeContextCurrent will generate a BadAccess error.
> - with:
> - If ctx is an indirect context current to some other thread,
> - then glXMakeContextCurrent will generate a BadAccess error.
> -
> - Replace the following sentence from section 3.5 Rendering Contexts:
> - If ctx is current to some other thread, then
> - glXMakeCurrent will generate a BadAccess error.
> - with:
> - If ctx is an indirect context current to some other thread,
> - then glXMakeCurrent will generate a BadAccess error.
> -
> -GLX Protocol
> -
> - None. The GLX extension only extends to direct rendering contexts.
> -
> -Errors
> -
> - None.
> -
> -New State
> -
> - None.
> -
> -Issues
> -
> - (1) What happens if the app binds a context/drawable in multiple
> - threads, then binds a different context/thread in one of them?
> -
> - As with binding a new context from the current thread, the old
> - context's refcount is reduced and the new context's refcount is
> - increased.
> -
> - (2) What happens if the app binds a context/drawable in multiple
> - threads, then binds None/None in one of them?
> -
> - The GLX context is unreferenced from that thread, and the other
> - threads retain their GLX context binding.
> -
> - (3) What happens if the app binds a context/drawable in 7 threads,
> - then destroys the context in one of them?
> -
> - As with GLX context destruction previously, the XID is destroyed
> - but the context remains usable by threads that have the context
> - current.
> -
> - (4) What happens if the app binds a new drawable/readable with
> - glXMakeCurrent() when it is already bound to another thread?
> -
> - The context becomes bound to the new drawable/readable, and
> - further rendering in either thread will use the new
> - drawable/readable.
> -
> - (5) What requirements should be placed on the user managing contexts
> - from multiple threads?
> -
> - The intention is to allow multithreaded access to the GL at the
> - minimal performance cost, so requiring that the GL do general
> - synchronization (beyond that already required by context sharing)
> - is not an option, and synchronizing of GL's access to the GL
> - context between multiple threads is left to the application to do
> - across GL calls. However, it would be unfortunate for a library
> - doing multithread_makecurrent to require that other libraries
> - share in synchronization for binding of their own contexts, so the
> - refcounting of the contexts is required to be threadsafe.
> -
> - (6) Does this apply to indirect contexts?
> -
> - This was ignored in the initial revision of the spec. Behavior
> - for indirect contexts is left as-is.
> -
> -Revision History
> -
> - 20 November 2009 Eric Anholt - initial specification
> - 22 November 2009 Eric Anholt - added issues from Ian Romanick.
> - 3 February 2011 Eric Anholt - updated with resolution to issues 1-3
> - 3 February 2011 Eric Anholt - added issue 4, 5
> - 21 February 2011 Eric Anholt - Include glXMakeCurrent() sentence
> - along with glXMakeContextCurrent() for removal.
> diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
> index 0d29e56..a8b542f 100644
> --- a/src/glx/glxclient.h
> +++ b/src/glx/glxclient.h
> @@ -426,23 +426,23 @@ struct glx_context
> * True core GL version supported by the server. This is the raw value
> * returned by the server, and it may not reflect what is actually
> * supported (or reported) by the client-side library.
> */
> /*@{ */
> int server_major; /**< Major version number. */
> int server_minor; /**< Minor version number. */
> /*@} */
>
> /**
> - * Number of threads we're currently current in.
> + * Thread ID we're currently current in. Zero if none.
> */
> - unsigned long thread_refcount;
> + unsigned long thread_id;
>
> char gl_extension_bits[__GL_EXT_BYTES];
> };
>
> extern Bool
> glx_context_init(struct glx_context *gc,
> struct glx_screen *psc, struct glx_config *fbconfig);
>
> #define __glXSetError(gc,code) \
> if (!(gc)->error) { \
> diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
> index d119326..7299988 100644
> --- a/src/glx/glxcurrent.c
> +++ b/src/glx/glxcurrent.c
> @@ -195,70 +195,68 @@ MakeContextCurrent(Display * dpy, GLXDrawable draw,
> /* Make sure that the new context has a nonzero ID. In the request,
> * a zero context ID is used only to mean that we bind to no current
> * context.
> */
> if ((gc != NULL) && (gc->xid == None)) {
> return GL_FALSE;
> }
>
> _glapi_check_multithread();
>
> - __glXLock();
> - if (oldGC == gc &&
> - gc->currentDrawable == draw && gc->currentReadable == read) {
> - __glXUnlock();
> - return True;
> + if (gc != NULL && gc->thread_id != 0 &&
> + gc->thread_id != _glthread_GetID()) {
> + __glXGenerateError(dpy, gc->xid, BadAccess, X_GLXMakeContextCurrent);
> + return False;
> }
>
> + if (oldGC == gc && gc->currentDrawable == draw &&
> + gc->currentReadable == read)
> + return True;
> +
> if (oldGC != &dummyContext) {
> - if (--oldGC->thread_refcount == 0) {
> - oldGC->vtable->unbind(oldGC, gc);
> - oldGC->currentDpy = 0;
> - }
> + oldGC->vtable->unbind(oldGC, gc);
> + oldGC->currentDpy = 0;
> + oldGC->thread_id = 0;
> }
>
> if (gc) {
> /* Attempt to bind the context. We do this before mucking with
> * gc and __glXSetCurrentContext to properly handle our state in
> * case of an error.
> *
> * If an error occurs, set the Null context since we've already
> * blown away our old context. The caller is responsible for
> * figuring out how to handle setting a valid context.
> */
> if (gc->vtable->bind(gc, oldGC, draw, read) != Success) {
> __glXSetCurrentContextNull();
> __glXUnlock();
> __glXGenerateError(dpy, None, GLXBadContext, X_GLXMakeContextCurrent);
> return GL_FALSE;
> }
>
> - if (gc->thread_refcount == 0) {
> - gc->currentDpy = dpy;
> - gc->currentDrawable = draw;
> - gc->currentReadable = read;
> - }
> - gc->thread_refcount++;
> + gc->currentDpy = dpy;
> + gc->currentDrawable = draw;
> + gc->currentReadable = read;
> + gc->thread_id = _glthread_GetID();
> __glXSetCurrentContext(gc);
> } else {
> __glXSetCurrentContextNull();
> }
>
> - if (oldGC->thread_refcount == 0 && oldGC != &dummyContext && oldGC->xid == None) {
> + if (oldGC != &dummyContext && oldGC->xid == None && oldGC != gc) {
> /* We are switching away from a context that was
> * previously destroyed, so we need to free the memory
> * for the old handle. */
> oldGC->vtable->destroy(oldGC);
> }
>
> - __glXUnlock();
> -
> /* The indirect vertex array state must to be initialised after we
> * have setup the context, as it needs to query server attributes.
> */
> if (gc && !gc->isDirect) {
> __GLXattribute *state = gc->client_state_private;
> if (state && state->array_state == NULL) {
> glGetString(GL_EXTENSIONS);
> glGetString(GL_VERSION);
> __glXInitVertexArrayState(gc);
> }
> diff --git a/src/glx/glxextensions.c b/src/glx/glxextensions.c
> index 22b078c..f17ede3 100644
> --- a/src/glx/glxextensions.c
> +++ b/src/glx/glxextensions.c
> @@ -141,21 +141,21 @@ static const struct extension_info known_glx_extensions[] = {
> { GLX(ARB_multisample), VER(1,4), Y, Y, N, N },
> { GLX(ATI_pixel_format_float), VER(0,0), N, N, N, N },
> { GLX(EXT_import_context), VER(0,0), Y, Y, N, N },
> { GLX(EXT_visual_info), VER(0,0), Y, Y, N, N },
> { GLX(EXT_visual_rating), VER(0,0), Y, Y, N, N },
> { GLX(EXT_fbconfig_packed_float), VER(0,0), Y, Y, N, N },
> { GLX(EXT_framebuffer_sRGB), VER(0,0), Y, Y, N, N },
> { GLX(EXT_create_context_es2_profile), VER(0,0), Y, N, N, N },
> { GLX(EXT_create_context_es_profile), VER(0,0), Y, N, N, N },
> { GLX(MESA_copy_sub_buffer), VER(0,0), Y, N, N, N },
> - { GLX(MESA_multithread_makecurrent),VER(0,0), Y, N, Y, N },
> + { GLX(MESA_multithread_makecurrent),VER(0,0), N, N, N, N },
> { GLX(MESA_query_renderer), VER(0,0), Y, N, N, Y },
> { GLX(MESA_swap_control), VER(0,0), Y, N, N, Y },
> { GLX(NV_float_buffer), VER(0,0), N, N, N, N },
> { GLX(OML_swap_method), VER(0,0), Y, Y, N, N },
> { GLX(OML_sync_control), VER(0,0), Y, N, N, Y },
> { GLX(SGI_make_current_read), VER(1,3), Y, N, N, N },
> { GLX(SGI_swap_control), VER(0,0), Y, N, N, N },
> { GLX(SGI_video_sync), VER(0,0), Y, N, N, Y },
> { GLX(SGIS_multisample), VER(0,0), Y, Y, N, N },
> { GLX(SGIX_fbconfig), VER(1,3), Y, Y, N, N },
> diff --git a/src/mapi/mapi_glapi.c b/src/mapi/mapi_glapi.c
> index 9f02edb..b8f2980 100644
> --- a/src/mapi/mapi_glapi.c
> +++ b/src/mapi/mapi_glapi.c
> @@ -238,28 +238,24 @@ _glapi_new_nop_table(unsigned num_entries)
> }
> return table;
> }
>
> void
> _glapi_set_nop_handler(_glapi_nop_handler_proc func)
> {
> table_set_noop_handler(func);
> }
>
> -/**
> - * This is a deprecated function which should not be used anymore.
> - * It's only present to satisfy linking with older versions of libGL.
> - */
> unsigned long
> _glthread_GetID(void)
> {
> - return 0;
> + return u_thread_self();
> }
>
> void
> _glapi_noop_enable_warnings(unsigned char enable)
> {
> }
>
> void
> _glapi_set_warning_func(_glapi_proc func)
> {
>
More information about the mesa-dev
mailing list