Skip to content

Replace Win32 PAL implementation with minipal implementation#115858

Merged
AaronRobinsonMSFT merged 42 commits into
dotnet:mainfrom
AaronRobinsonMSFT:replace_win32_critsec
May 29, 2025
Merged

Replace Win32 PAL implementation with minipal implementation#115858
AaronRobinsonMSFT merged 42 commits into
dotnet:mainfrom
AaronRobinsonMSFT:replace_win32_critsec

Conversation

@AaronRobinsonMSFT

@AaronRobinsonMSFT AaronRobinsonMSFT commented May 22, 2025

Copy link
Copy Markdown
Member

This PR removes the PAL Win32 emulation of CRITICAL_SECTION. The emulation logic is unnecessarily complicated and used both a pthread mutex and condition variable. For CoreCLR purposes a mutex is sufficient.

A new minipal API was introduced, minipal_critsect. This uses the Win32 CRITICAL_SECTION on the Windows platform and pthread_mutex_t on all other platforms. This implementation aligns with a private "critical section" defined for the GC and native AOT. All now use this new API.

The CRITICAL_SECTION associated APIs and tests in the CoreCLR PAL have been removed.

@huoyaoyuan

Copy link
Copy Markdown
Member

FWIW, I tested std::mutex and mtx_t on Windows. std::mutex is about the same speed of CRITICAL_SECTION, and mtx_t is 2x slower. We are unable to use std::mutex because it's not noexcept.

Comment thread src/native/minipal/critsect.c Outdated
Remove any non-running tests requiring CRITICAL_SECTION
Remove unused PAL header referencing CRITICAL_SECTION (internal.h)
@AaronRobinsonMSFT

AaronRobinsonMSFT commented May 22, 2025

Copy link
Copy Markdown
Member Author

FWIW, I tested std::mutex and mtx_t on Windows. std::mutex is about the same speed of CRITICAL_SECTION, and mtx_t is 2x slower. We are unable to use std::mutex because it's not noexcept.

It isn't just because of noexcept. On Windows std::mutex will pull in the concrt, which is a huge dependency.

mtx_t

clang on macOS doesn't support C11 threads. It isn't an option in any case.

Comment thread src/native/minipal/critsect.h Outdated
Comment thread src/native/minipal/critsect.h Outdated
Comment thread src/coreclr/inc/crosscomp.h Outdated
Comment thread src/coreclr/inc/crosscomp.h Outdated
Comment thread src/coreclr/inc/crosscomp.h Outdated
Comment thread src/coreclr/gc/env/gcenv.base.h Outdated
Comment thread src/coreclr/nativeaot/Runtime/PalRedhawk.h Outdated
Comment thread src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.h Outdated
Comment thread src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.h Outdated
Comment thread src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp Outdated
Comment thread src/coreclr/nativeaot/Runtime/rhassert.h Outdated
Comment thread src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp Outdated
Comment thread src/coreclr/nativeaot/Runtime/rhassert.h Outdated
Comment thread src/coreclr/inc/crosscomp.h Outdated
Comment thread src/native/minipal/critsect.h Outdated
@richlander

Copy link
Copy Markdown
Member

Related: #47700

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants