From 54466bc4412acf33a59af59d9eadde54c22b2ebe Mon Sep 17 00:00:00 2001 From: Michael Lentine Date: Tue, 27 Jan 2015 09:01:03 -0800 Subject: [PATCH] Remove lock around ref count check in terminate. Replace the global lock in terminate for the ref count check with atomics and a local lock. Bug: 19072064 Change-Id: I0cfd6564e227a75b8387a8788b16381d5bc2cc88 --- opengl/libs/EGL/egl_display.cpp | 293 +++++++++++++++++--------------- opengl/libs/EGL/egl_display.h | 4 +- 2 files changed, 162 insertions(+), 135 deletions(-) diff --git a/opengl/libs/EGL/egl_display.cpp b/opengl/libs/EGL/egl_display.cpp index 7784ca6ac..ec59235b4 100644 --- a/opengl/libs/EGL/egl_display.cpp +++ b/opengl/libs/EGL/egl_display.cpp @@ -57,7 +57,7 @@ static bool findExtension(const char* exts, const char* name, size_t nameLen) { egl_display_t egl_display_t::sDisplay[NUM_DISPLAYS]; egl_display_t::egl_display_t() : - magic('_dpy'), finishOnSwap(false), traceGpuCompletion(false), refs(0) { + magic('_dpy'), finishOnSwap(false), traceGpuCompletion(false), refs(0), eglIsInitialized(false) { } egl_display_t::~egl_display_t() { @@ -120,163 +120,188 @@ EGLDisplay egl_display_t::getDisplay(EGLNativeDisplayType display) { EGLBoolean egl_display_t::initialize(EGLint *major, EGLint *minor) { - Mutex::Autolock _l(lock); + { + Mutex::Autolock _rf(refLock); + + refs++; + if (refs > 1) { + if (major != NULL) + *major = VERSION_MAJOR; + if (minor != NULL) + *minor = VERSION_MINOR; + while(!eglIsInitialized) refCond.wait(refLock); + return EGL_TRUE; + } + + while(eglIsInitialized) refCond.wait(refLock); + } + + { + Mutex::Autolock _l(lock); + +#if EGL_TRACE + + // Called both at early_init time and at this time. (Early_init is pre-zygote, so + // the information from that call may be stale.) + initEglTraceLevel(); + initEglDebugLevel(); + +#endif + + setGLHooksThreadSpecific(&gHooksNoContext); + + // initialize each EGL and + // build our own extension string first, based on the extension we know + // and the extension supported by our client implementation + + egl_connection_t* const cnx = &gEGLImpl; + cnx->major = -1; + cnx->minor = -1; + if (cnx->dso) { + EGLDisplay idpy = disp.dpy; + if (cnx->egl.eglInitialize(idpy, &cnx->major, &cnx->minor)) { + //ALOGD("initialized dpy=%p, ver=%d.%d, cnx=%p", + // idpy, cnx->major, cnx->minor, cnx); + + // display is now initialized + disp.state = egl_display_t::INITIALIZED; + + // get the query-strings for this display for each implementation + disp.queryString.vendor = cnx->egl.eglQueryString(idpy, + EGL_VENDOR); + disp.queryString.version = cnx->egl.eglQueryString(idpy, + EGL_VERSION); + disp.queryString.extensions = cnx->egl.eglQueryString(idpy, + EGL_EXTENSIONS); + disp.queryString.clientApi = cnx->egl.eglQueryString(idpy, + EGL_CLIENT_APIS); + + } else { + ALOGW("eglInitialize(%p) failed (%s)", idpy, + egl_tls_t::egl_strerror(cnx->egl.eglGetError())); + } + } + + // the query strings are per-display + mVendorString.setTo(sVendorString); + mVersionString.setTo(sVersionString); + mClientApiString.setTo(sClientApiString); + + mExtensionString.setTo(gBuiltinExtensionString); + char const* start = gExtensionString; + char const* end; + do { + // find the space separating this extension for the next one + end = strchr(start, ' '); + if (end) { + // length of the extension string + const size_t len = end - start; + if (len) { + // NOTE: we could avoid the copy if we had strnstr. + const String8 ext(start, len); + if (findExtension(disp.queryString.extensions, ext.string(), + len)) { + mExtensionString.append(start, len+1); + } + } + // process the next extension string, and skip the space. + start = end + 1; + } + } while (end); + + egl_cache_t::get()->initialize(this); + + char value[PROPERTY_VALUE_MAX]; + property_get("debug.egl.finish", value, "0"); + if (atoi(value)) { + finishOnSwap = true; + } + + property_get("debug.egl.traceGpuCompletion", value, "0"); + if (atoi(value)) { + traceGpuCompletion = true; + } - if (refs > 0) { if (major != NULL) *major = VERSION_MAJOR; if (minor != NULL) *minor = VERSION_MINOR; - refs++; - return EGL_TRUE; + + mHibernation.setDisplayValid(true); } -#if EGL_TRACE - - // Called both at early_init time and at this time. (Early_init is pre-zygote, so - // the information from that call may be stale.) - initEglTraceLevel(); - initEglDebugLevel(); - -#endif - - setGLHooksThreadSpecific(&gHooksNoContext); - - // initialize each EGL and - // build our own extension string first, based on the extension we know - // and the extension supported by our client implementation - - egl_connection_t* const cnx = &gEGLImpl; - cnx->major = -1; - cnx->minor = -1; - if (cnx->dso) { - EGLDisplay idpy = disp.dpy; - if (cnx->egl.eglInitialize(idpy, &cnx->major, &cnx->minor)) { - //ALOGD("initialized dpy=%p, ver=%d.%d, cnx=%p", - // idpy, cnx->major, cnx->minor, cnx); - - // display is now initialized - disp.state = egl_display_t::INITIALIZED; - - // get the query-strings for this display for each implementation - disp.queryString.vendor = cnx->egl.eglQueryString(idpy, - EGL_VENDOR); - disp.queryString.version = cnx->egl.eglQueryString(idpy, - EGL_VERSION); - disp.queryString.extensions = cnx->egl.eglQueryString(idpy, - EGL_EXTENSIONS); - disp.queryString.clientApi = cnx->egl.eglQueryString(idpy, - EGL_CLIENT_APIS); - - } else { - ALOGW("eglInitialize(%p) failed (%s)", idpy, - egl_tls_t::egl_strerror(cnx->egl.eglGetError())); - } + { + Mutex::Autolock _rf(refLock); + eglIsInitialized = true; + refCond.broadcast(); } - // the query strings are per-display - mVendorString.setTo(sVendorString); - mVersionString.setTo(sVersionString); - mClientApiString.setTo(sClientApiString); - - mExtensionString.setTo(gBuiltinExtensionString); - char const* start = gExtensionString; - char const* end; - do { - // find the space separating this extension for the next one - end = strchr(start, ' '); - if (end) { - // length of the extension string - const size_t len = end - start; - if (len) { - // NOTE: we could avoid the copy if we had strnstr. - const String8 ext(start, len); - if (findExtension(disp.queryString.extensions, ext.string(), - len)) { - mExtensionString.append(start, len+1); - } - } - // process the next extension string, and skip the space. - start = end + 1; - } - } while (end); - - egl_cache_t::get()->initialize(this); - - char value[PROPERTY_VALUE_MAX]; - property_get("debug.egl.finish", value, "0"); - if (atoi(value)) { - finishOnSwap = true; - } - - property_get("debug.egl.traceGpuCompletion", value, "0"); - if (atoi(value)) { - traceGpuCompletion = true; - } - - refs++; - if (major != NULL) - *major = VERSION_MAJOR; - if (minor != NULL) - *minor = VERSION_MINOR; - - mHibernation.setDisplayValid(true); - return EGL_TRUE; } EGLBoolean egl_display_t::terminate() { - Mutex::Autolock _l(lock); + { + Mutex::Autolock _rl(refLock); + if (refs == 0) { + /* + * From the EGL spec (3.2): + * "Termination of a display that has already been terminated, + * (...), is allowed, but the only effect of such a call is + * to return EGL_TRUE (...) + */ + return EGL_TRUE; + } - if (refs == 0) { - /* - * From the EGL spec (3.2): - * "Termination of a display that has already been terminated, - * (...), is allowed, but the only effect of such a call is - * to return EGL_TRUE (...) - */ - return EGL_TRUE; - } - - // this is specific to Android, display termination is ref-counted. - if (refs > 1) { + // this is specific to Android, display termination is ref-counted. refs--; - return EGL_TRUE; + if (refs > 0) { + return EGL_TRUE; + } } EGLBoolean res = EGL_FALSE; - egl_connection_t* const cnx = &gEGLImpl; - if (cnx->dso && disp.state == egl_display_t::INITIALIZED) { - if (cnx->egl.eglTerminate(disp.dpy) == EGL_FALSE) { - ALOGW("eglTerminate(%p) failed (%s)", disp.dpy, - egl_tls_t::egl_strerror(cnx->egl.eglGetError())); + + { + Mutex::Autolock _l(lock); + + egl_connection_t* const cnx = &gEGLImpl; + if (cnx->dso && disp.state == egl_display_t::INITIALIZED) { + if (cnx->egl.eglTerminate(disp.dpy) == EGL_FALSE) { + ALOGW("eglTerminate(%p) failed (%s)", disp.dpy, + egl_tls_t::egl_strerror(cnx->egl.eglGetError())); + } + // REVISIT: it's unclear what to do if eglTerminate() fails + disp.state = egl_display_t::TERMINATED; + res = EGL_TRUE; } - // REVISIT: it's unclear what to do if eglTerminate() fails - disp.state = egl_display_t::TERMINATED; - res = EGL_TRUE; + + mHibernation.setDisplayValid(false); + + // Reset the extension string since it will be regenerated if we get + // reinitialized. + mExtensionString.setTo(""); + + // Mark all objects remaining in the list as terminated, unless + // there are no reference to them, it which case, we're free to + // delete them. + size_t count = objects.size(); + ALOGW_IF(count, "eglTerminate() called w/ %d objects remaining", count); + for (size_t i=0 ; idestroy(); + } + + // this marks all object handles are "terminated" + objects.clear(); } - mHibernation.setDisplayValid(false); - - // Reset the extension string since it will be regenerated if we get - // reinitialized. - mExtensionString.setTo(""); - - // Mark all objects remaining in the list as terminated, unless - // there are no reference to them, it which case, we're free to - // delete them. - size_t count = objects.size(); - ALOGW_IF(count, "eglTerminate() called w/ %d objects remaining", count); - for (size_t i=0 ; idestroy(); + { + Mutex::Autolock _rl(refLock); + eglIsInitialized = false; + refCond.broadcast(); } - // this marks all object handles are "terminated" - objects.clear(); - - refs--; return res; } diff --git a/opengl/libs/EGL/egl_display.h b/opengl/libs/EGL/egl_display.h index 87f27f800..4949ef212 100644 --- a/opengl/libs/EGL/egl_display.h +++ b/opengl/libs/EGL/egl_display.h @@ -131,7 +131,9 @@ private: void leave() { return mHibernation.decWakeCount(HibernationMachine::WEAK); } uint32_t refs; - mutable Mutex lock; + bool eglIsInitialized; + mutable Mutex lock, refLock; + mutable Condition refCond; SortedVector objects; String8 mVendorString; String8 mVersionString;