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

Use SRWLOCK_INIT to statically initialize stl_critical_section_win7::m_srw_lock #3522

Merged

Conversation

StephanTLavavej
Copy link
Member

This is a minor (but strict) performance improvement as it avoids a Windows API call.

Although this changes separately compiled code, it doesn't affect the redist's interface, and we don't have to wait for the redist to be "unlocked". stl/src/primitives.hpp is built into the STL's DLLs and static LIBs, so there are no mix-and-match concerns with user code.

We use SRWLOCK_INIT everywhere else:

SRWLOCK _Lock = SRWLOCK_INIT;

static SRWLOCK _Mtx = SRWLOCK_INIT;

SRWLOCK _Shared_ptr_lock = SRWLOCK_INIT;

inline static SRWLOCK srw = SRWLOCK_INIT;

…n7::m_srw_lock.

This doesn't affect the redist's interface - stl/src/primitives.hpp is
built into the DLL or static LIB, so there's no mix-and-match concern.

We use SRWLOCK_INIT everywhere now.
@StephanTLavavej StephanTLavavej added the performance Must go faster label Mar 3, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner March 3, 2023 19:04
stl/src/primitives.hpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this Mar 7, 2023
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 557718f into microsoft:main Mar 7, 2023
@StephanTLavavej StephanTLavavej deleted the stl-cleanups-srwlock branch March 7, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants