src/libANGLE/SharedContextMutex.cpp


Log

Author Commit Date CI Message
Igor Nazarov d29f946d 2023-07-26T19:55:56 Reland "Fix ExternalImageTarget EGLImage race" This is a reland of commit 8aa3ca9d177c0ed54926b769de7d0bce0f8482d3 Previous submission was reverted because of a failure in Android's "testDrawingHardwareBitmapNotLeaking" test. Bug was not in the reverted CL itself and is fixed by CL: https://chromium-review.googlesource.com/c/angle/angle/+/4733831 Original change's description: > Fix ExternalImageTarget EGLImage race > > Race may happen when ExternalImageTarget EGLImage is destroyed while its > GLES Texture/Renderbuffer target is modified/destroyed. > > Fixed by providing `egl::Image` with `egl::ContextMutex` even when > `context` is `nullptr`. > > This CL also changes `SharedContextMutex` merging rules when `mRank` is > equal - now priority goes to the `lockedMutex`. This is done to prevent > unnecessary `mRoot` update of Context mutex when merging with > `egl::Image` only mutex. > > Bug: angleproject:6957 > Change-Id: I823e53b98f70ed3eaca191e8be5b168dc07899f6 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4720835 > Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> > Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Bug: angleproject:6957 Change-Id: Ia698dbea64b8d51ea78f3af6558efc7266f8bdc8 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4732892 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Igor Nazarov d61a50c1 2023-07-31T15:09:08 Add "angle_enable_context_mutex_recursion" build option Option is for Android Vulkan. Fixes the recursion problem: #00 pc 000000000004d69c /apex/com.android.runtime/lib64/bionic/libc.so (syscall+28) (BuildId: dda896312b8ac9c639f6aeb0175b8a0a) #01 pc 0000000000051e84 /apex/com.android.runtime/lib64/bionic/libc.so (__futex_wait_ex(void volatile*, bool, int, bool, timespec const*)+144) (BuildId: dda896312b8ac9c639f6aeb0175b8a0a) #02 pc 00000000000b41b4 /apex/com.android.runtime/lib64/bionic/libc.so (NonPI::MutexLockWithTimeout(pthread_mutex_internal_t*, bool, timespec const*)+236) (BuildId: dda896312b8ac9c639f6aeb0175b8a0a) #03 pc 00000000003d8274 /vendor/lib64/egl/libGLESv2_angle.so (std::__Cr::mutex::lock()+12) (BuildId: 203fee8f1cfe9c18) #04 pc 0000000000336494 /vendor/lib64/egl/libGLESv2_angle.so (egl::SharedContextMutex<std::__Cr::mutex>::doLock()+132) (BuildId: 203fee8f1cfe9c18) #05 pc 000000000033668c /vendor/lib64/egl/libGLESv2_angle.so (egl::SharedContextMutex<std::__Cr::mutex>::lock()+124) (BuildId: 203fee8f1cfe9c18) #06 pc 00000000001d6e08 /vendor/lib64/egl/libGLESv2_angle.so (EGL_ClientWaitSyncKHR+100) (BuildId: 203fee8f1cfe9c18) #07 pc 00000000000674f0 /system/lib64/libEGL.so (android::eglClientWaitSyncKHRImpl(void*, void*, int, unsigned long)+112) (BuildId: c4698b8b366f6519dbd7bd3a5f6239f2) #08 pc 0000000000086648 /system/lib64/libgui.so (android::BufferQueueProducer::dequeueBuffer(int*, android::sp<android::Fence>*, unsigned int, unsigned int, int, unsigned long, unsigned long*, android::FrameEventHistoryDelta*)+3240) (BuildId: ff35b91736084ee7c49efe908fb9f8f3) #09 pc 00000000000e24fc /system/lib64/libgui.so (android::Surface::dequeueBuffer(ANativeWindowBuffer**, int*)+392) (BuildId: ff35b91736084ee7c49efe908fb9f8f3) #10 pc 0000000000755770 /system/lib64/libhwui.so (android::uirenderer::renderthread::ReliableSurface::hook_dequeueBuffer(ANativeWindow*, int (*)(ANativeWindow*, ANativeWindowBuffer**, int*), void*, ANativeWindowBuffer**, int*)+80) (BuildId: 0ab7d7584ac800860c4d180557441d1b) #11 pc 00000000000e0b48 /system/lib64/libgui.so (android::Surface::hook_dequeueBuffer(ANativeWindow*, ANativeWindowBuffer**, int*)+92) (BuildId: ff35b91736084ee7c49efe908fb9f8f3) #12 pc 000000000002cc8c /system/lib64/libvulkan.so (vulkan::driver::AcquireNextImageKHR(VkDevice_T*, VkSwapchainKHR_T*, unsigned long, VkSemaphore_T*, VkFence_T*, unsigned int*)+280) (BuildId: 0c72f8685858f73fbb13fd68d401bba5) #13 pc 0000000000266f08 /vendor/lib64/egl/libGLESv2_angle.so (rx::(anonymous namespace)::TryAcquireNextImageUnlocked(VkDevice_T*, VkSwapchainKHR_T*, rx::impl::ImageAcquireOperation*)+128) (BuildId: 203fee8f1cfe9c18) #14 pc 000000000026495c /vendor/lib64/egl/libGLESv2_angle.so (rx::WindowSurfaceVk::acquireNextSwapchainImage(rx::vk::Context*)+140) (BuildId: 203fee8f1cfe9c18) #15 pc 0000000000265800 /vendor/lib64/egl/libGLESv2_angle.so (rx::WindowSurfaceVk::doDeferredAcquireNextImageWithUsableSwapchain(gl::Context const*)+148) (BuildId: 203fee8f1cfe9c18) #16 pc 0000000000267bb4 /vendor/lib64/egl/libGLESv2_angle.so (rx::WindowSurfaceVk::getBufferAge(gl::Context const*, int*)+196) (BuildId: 203fee8f1cfe9c18) #17 pc 000000000034143c /vendor/lib64/egl/libGLESv2_angle.so (egl::Surface::getBufferAge(gl::Context const*, int*)+36) (BuildId: 203fee8f1cfe9c18) #18 pc 000000000036c354 /vendor/lib64/egl/libGLESv2_angle.so (egl::QuerySurfaceAttrib(egl::Display const*, gl::Context const*, egl::Surface*, int, int*)+812) (BuildId: 203fee8f1cfe9c18) #19 pc 00000000001d1674 /vendor/lib64/egl/libGLESv2_angle.so (egl::QuerySurface(egl::Thread*, egl::Display*, egl::SurfaceID, int, int*)+176) (BuildId: 203fee8f1cfe9c18) #20 pc 00000000001d3658 /vendor/lib64/egl/libGLESv2_angle.so (EGL_QuerySurface+200) (BuildId: 203fee8f1cfe9c18) Happens only on platforms that use "EGL_KHR_fence_sync" instead of "EGL_ANDROID_native_fence_sync" for synchronization. Happens only on specific Surface types, that using `EGLConsumer`/`GLConsumer`, such as `SurfaceTexture`. The Android's "testDrawingHardwareBitmapNotLeaking" test may be used for testing the issue. Note: in order to reproduce, `SingleContextMutex` must not be used by context. The "Fix ExternalImageTarget EGLImage race" CL will achieve that. To check what extension is used, call: adb shell "dumpsys SurfaceFlinger | grep 'Sync configuration'" Possible results: Sync configuration: [using: EGL_KHR_fence_sync EGL_KHR_wait_sync] Sync configuration: [using: EGL_ANDROID_native_fence_sync EGL_KHR_wait_sync] Bug: angleproject:4354 Change-Id: I915a2c026b59af0a2098ae6b3300f773b29cbfbb Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4733831 Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Roman Lavrov 179924cb 2023-07-27T20:45:26 Revert "Fix ExternalImageTarget EGLImage race" This reverts commit 8aa3ca9d177c0ed54926b769de7d0bce0f8482d3. Reason for revert: Confirmed to break Android's testDrawingHardwareBitmapNotLeaking in this single-commit roll: https://r.android.com/2679397 Original change's description: > Fix ExternalImageTarget EGLImage race > > Race may happen when ExternalImageTarget EGLImage is destroyed while its > GLES Texture/Renderbuffer target is modified/destroyed. > > Fixed by providing `egl::Image` with `egl::ContextMutex` even when > `context` is `nullptr`. > > This CL also changes `SharedContextMutex` merging rules when `mRank` is > equal - now priority goes to the `lockedMutex`. This is done to prevent > unnecessary `mRoot` update of Context mutex when merging with > `egl::Image` only mutex. > > Bug: angleproject:6957 > Change-Id: I823e53b98f70ed3eaca191e8be5b168dc07899f6 > Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4720835 > Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> > Commit-Queue: Igor Nazarov <i.nazarov@samsung.com> Bug: angleproject:6957 Change-Id: I860a8bfd6dd66eb549045391755a83483109ebbb No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4727621 Commit-Queue: Roman Lavrov <romanl@google.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Igor Nazarov 8aa3ca9d 2023-07-26T19:55:56 Fix ExternalImageTarget EGLImage race Race may happen when ExternalImageTarget EGLImage is destroyed while its GLES Texture/Renderbuffer target is modified/destroyed. Fixed by providing `egl::Image` with `egl::ContextMutex` even when `context` is `nullptr`. This CL also changes `SharedContextMutex` merging rules when `mRank` is equal - now priority goes to the `lockedMutex`. This is done to prevent unnecessary `mRoot` update of Context mutex when merging with `egl::Image` only mutex. Bug: angleproject:6957 Change-Id: I823e53b98f70ed3eaca191e8be5b168dc07899f6 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4720835 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 3545ae0c 2023-06-09T16:11:47 Add isContextMutexStateConsistent() ASSERT Added `gl::Context::isContextMutexStateConsistent()` method. This method is primarily used to check if "SharedContextMutex" activation worked successfully. It is automatically called in the updated `ScopedContextMutexLock` before unlocking. This is to catch possible errors using ASSERT during normal ANGLE operation in applications and tests. The `ScopedContextMutexLock` is now also used instead of the `std::lock_guard<egl::ContextMutex>` in the `SCOPED_SHARE_CONTEXT_LOCK`. No performance regression observed. Important note: `lockAndActivateSharedContextMutex()` is NOT 100% safe regardless of the `kActivationDelayMicro` value, so `ASSERT` may still fail. However, failure does not necessary mean that there will be an undefined behavior, it means that UB might happen. Bug: angleproject:6957 Bug: chromium:1336126 Change-Id: Iee7357fede0d37fa315fe2cc7d27a4e30a304194 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4610227 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Igor Nazarov <i.nazarov@samsung.com>
Igor Nazarov 36c3e0f5 2023-01-17T17:42:59 Implement "Shared Context Mutex" functionality. Existing implementation uses single `GlobalMutex` for - EGL calls - GL calls for Contexts with concurrent access. This CL introduces abstract `egl::ContextMutex` with two implementations: - SingleContextMutex; - SharedContextMutex<Mutex>; Note: `std::mutex` is used in this commit. It is very easy to change mutex type either at compile-time or at run-time (single type per Display). When Context: - is not Shared; - does not use `EGLImage`s; - does not use EGL_DISPLAY_TEXTURE_SHARE_GROUP_ANGLE - does not use EGL_DISPLAY_SEMAPHORE_SHARE_GROUP_ANGLE then it will be using `SingleContextMutex` with minimal overhead. Before such Context is used as `shareContext` or uses `EGLImage` its mutex replaced by `SharedContextMutex<Mutex>`. The `GlobalMutex` is only used for EGL calls, while `egl::ContextMutex` implementations for GL calls. Because some EGL calls use Context, explicit `egl::ContextMutex` lock is required. This is implemented by generating "egl_context_mutex_autogen.h" header, and insertion of `ANGLE_EGL_SCOPED_CONTEXT_LOCK()` macro before `ANGLE_EGL_VALIDATE()` in each EGL entry point. Implementation in "egl_context_lock_impl.h" returns lock for required APIs. Special cases of `egl::ContextMutex` lock handled separately. `std::unique_lock<>` is not used for performance reasons. `egl::ContextMutex` explicitly locked when capturing EGL calls. Fixes EGLImage problem: https://chromium.googlesource.com/angle/angle/+/e18240d136d15e5cdfa4fa4a6355ca21c8d807b6 Mark contexts as shared when importing EGL images. Details: - EGLImage inherits Context's mutex when created. Mutex is used when the EGLImage accessed or destroyed. - When EGLImage is used in Context with other `egl::ContextMutex`, two mutexes are merged into one. - After the mutex merge, Context Groups will remain separate, but will not be able to run in parallel. Fixes race when checking `context->isShared()` in the `SCOPED_SHARE_CONTEXT_LOCK()` macro. One Context may start executing GL call while not "Shared", but become "Shared" inside the call. New (second) "Shared" Context may immediately start using GL and potentially corrupt some "Shared" state. Possible performance benefit: allows parallel execution in some cases, when single `GlobalMutex` would block. Important note: Process of replacing the `SingleContextMutex` by `SharedContextMutex<Mutex>` is not 100% safe. This mean that original Context may still be using `SingleContextMutex` after activating `SharedContextMutex<Mutex>`. However, this was always the case before introduction of this CL. Old `Context::mShared` member update was not synchronized in any way at all. In other words, this solution does not 100% fix the original problem. For 100% safe solution `SingleContextMutex` should not be used (always pass `SharedContextMutex<Mutex>` to the `gl::Context` constructor). See `lockAndActivateSharedContextMutex()` for more details. CL adds new build option: angle_enable_shared_context_mutex = true Behavior with other build options: - When: `angle_enable_shared_context_mutex` is disabled or `angle_enable_share_context_lock` is disabled or `angle_force_context_check_every_call` is enabled, Contexts will always have `SingleContextMutex`, however it will be only used in special cases. `SCOPED_SHARE_CONTEXT_LOCK()` will use `GlobalMutex` when applicable. - Otherwise, `SCOPED_SHARE_CONTEXT_LOCK()` will use `egl::ContextMutex`. Some GFXBench "1080p Driver Overhead 2 Offscreen" performance numbers. Tested on S906B (Samsung Galaxy S22+) on old ANGLE base: https://chromium.googlesource.com/angle/angle/+/807c94ea85e046c6f279d081d99f0fb1bcf1191a Capture/Replay: Adjust tests do adhere to capture limits Each test result is an average frame number from 6 runs. SingleContextMutex 6579 ( +0.13%) (old) GetContextLock() (mShared is false) 6570 Forced `mShared = true` or NOT using `SingleContextMutex`. SharedContextMutex<std::mutex> FORCE 5061 (-22.97%) (old) GetContextLock() FORCE 4766 (-27.46%) Bug: angleproject:6957 Bug: chromium:1336126 Change-Id: Idcd919f9d4bf482b9ae489bd8b4415ec96048e32 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4374545 Reviewed-by: Geoff Lang <geofflang@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>