Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[chrome.angle] Force Pure RISCV64 Support on Android #5

Conversation

McKnight22
Copy link

Utilize the definition of skip_secondary_abi_for_cq and android_secondary_abi_toolchain directly to support pure RISCV64 on Android.

Before RISCV64 introduced into Chromium, every 64-bit ARCH enables its own 32-bit support by default.
But for RISCV, Android announced only pure 64-bit would be supported.
So for Chromium RISCV64 on Android, one proposed solution based on the commit is to skip the legacy check logic for its 32-bit ABI.

Utilize the definition of skip_secondary_abi_for_cq and android_secondary_abi_toolchain directly to support pure RISCV64 on Android.

Before RISCV64 introduced into Chromium, every 64-bit ARCH enables its own 32-bit support by default.
But for RISCV, Android announced only pure 64-bit would be supported.
So for Chromium RISCV64 on Android, one proposed solution based on the commit is to skip the legacy check logic for its 32-bit ABI.

Signed-off-by: McKnight22 <[email protected]>
@unicornx
Copy link

unicornx commented Apr 3, 2023

This PR is a cleanup for final merge based off discussion on #4.

@unicornx unicornx merged commit 937ede4 into aosp-riscv:riscv64_109.0.5414.87_dev Apr 3, 2023
@unicornx
Copy link

unicornx commented Apr 3, 2023

This PR is related to aosp-riscv/chromium#30.

unicornx pushed a commit that referenced this pull request May 22, 2023
This CL improves performance compared to the existing code as well as
allows using non-std mutex implementations. Also acts as a base for
future changes.

CL adds new build option:
    angle_enable_global_mutex_recursion = is_android && angle_enable_vulkan

"mutex_recursion" work same way as `std::recursive_mutex` before. It
will help in situations when Vulkan API may return back to the ANGLE.
For example: RenderDoc layer EGL deadlock.
Automatic loading of "libVkLayer_GLES_RenderDoc.so" layer causes
deadlock in EGL.
Recursion stack:
    #1 pc 000000000029ea80  /vendor/lib64/egl/libGLESv2_angle.so (egl::GlobalMutexHelper::lock(int)+596)
    #2 pc 000000000029c59c  /vendor/lib64/egl/libGLESv2_angle.so (EGL_GetError+32)

    #4 pc 0000000000062368  /system/lib64/libEGL.so (eglQueryString+20)
    #5 pc 0000000000508fec  /data/local/debug/vulkan/libVkLayer_GLES_RenderDoc.so

    google#20 pc 0000000000016690  /system/lib64/libvulkan.so (vulkan::api::EnumerateInstanceLayerProperties(unsigned int*, VkLayerProperties*)+40)
    google#21 pc 00000000005aa030  /vendor/lib64/egl/libGLESv2_angle.so (rx::RendererVk::initialize(rx::DisplayVk*, egl::Display*, char const*, char const*)+292)
    google#26 pc 000000000029c7e8  /vendor/lib64/egl/libGLESv2_angle.so (EGL_Initialize+192)

Additionally, recursive mutex will partially solve Android
SurfaceTexture deadlock (angleproject:4354).

Some performance numbers for 1000'000 `eglGetError()` calls.

    Mutex                                       Time (ms.)

        Android S906B

    egl::GetGlobalMutex()(std::recursive_mutex)    41.4

    (Default)   GlobalMutex (std::recursive_mutex) 39.1

    (Recursive) GlobalMutex (std::mutex)           34.9
    (Debug)     GlobalMutex (std::mutex)           34.7
    (Default)   GlobalMutex (std::mutex)           34.4

        Windows

    egl::GetGlobalMutex()(std::recursive_mutex)    20.5

    (Default)   GlobalMutex (std::recursive_mutex) 20.0

    (Recursive) GlobalMutex (std::mutex)           21.9
    (Debug)     GlobalMutex (std::mutex)           20.5
    (Default)   GlobalMutex (std::mutex)           19.9

Note: Recursive GlobalMutex enabled only for Android Vulkan by default.
Original fix:
    https://chromium-review.googlesource.com/c/angle/angle/+/2029218

Bug: angleproject:8101
Bug: angleproject:4354
Test: angle_unittests --gtest_filter=GlobalMutexTest.*
Change-Id: I9e9d9b5c598ad1177ffa147ea690bd955946a712
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4401940
Commit-Queue: Igor Nazarov <[email protected]>
Reviewed-by: Charlie Lao <[email protected]>
Reviewed-by: Shahbaz Youssefi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants