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

Define _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR #21005

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

baijumeswani
Copy link
Contributor

@baijumeswani baijumeswani commented Jun 11, 2024

microsoft/STL#3824 introduces constexpr mutex. An older version of msvcp140.dll will lead to A dynamic link library (DLL) initialization routine failed.

This error can be encountered if using conda Python since conda packages msvc dlls and these are older right now.

This PR disables the constexpr mutex so that ort package can work with older msvc dlls.

Thanks @snnn for the discovery.

@baijumeswani baijumeswani requested review from snnn and yufenglee June 11, 2024 20:32
@tianleiwu tianleiwu merged commit 94aa21c into main Jun 12, 2024
170 checks passed
@tianleiwu tianleiwu deleted the baijumeswani/non-constexpr-mutex branch June 12, 2024 05:23
@sophies927 sophies927 added the triage:approved Approved for cherrypicks for release label Jun 12, 2024
@snnn
Copy link
Member

snnn commented Jun 13, 2024

It cannot completely solve the problem. It only adds compatibility in a way that the built binaries will be not just compatible with 17.10 runtime but also 17.8 runtime. Just these two. The latest msvc runtime dll in the latest conda is 17.9. So this change is enough for solving that particular problem.

@sophies927 sophies927 removed the triage:approved Approved for cherrypicks for release label Jun 17, 2024
baijumeswani added a commit that referenced this pull request Jun 20, 2024
microsoft/STL#3824 introduces constexpr mutex.
An older version of msvcp140.dll will lead to ```A dynamic link library
(DLL) initialization routine failed```.

This error can be encountered if using conda Python since conda packages
msvc dlls and these are older right now.

This PR disables the constexpr mutex so that ort package can work with
older msvc dlls.

Thanks @snnn for the discovery.
yf711 pushed a commit that referenced this pull request Jun 21, 2024
microsoft/STL#3824 introduces constexpr mutex.
An older version of msvcp140.dll will lead to ```A dynamic link library
(DLL) initialization routine failed```.

This error can be encountered if using conda Python since conda packages
msvc dlls and these are older right now.

This PR disables the constexpr mutex so that ort package can work with
older msvc dlls.

Thanks @snnn for the discovery.
snnn pushed a commit that referenced this pull request Jul 27, 2024
The change in #21005 works for directly building wheels with `build.py`,
but ort-nightly-directml wheels, as well as the 1.18.1 release of the
onnxruntime-directml python wheel, still do not work with conda since
they're built from the `py-win-gpu.yml` pipeline, which uses
`install_third_party_deps.ps1` to set compile flags.
snnn added a commit that referenced this pull request Oct 18, 2024
This was referenced Oct 18, 2024
snnn added a commit that referenced this pull request Oct 21, 2024
### Description
1. Remove the onnxruntime::OrtMutex class and replace it with
~absl::Mutex~ std::mutex.
2. After this change, most source files will not include <Windows.h>
indirectly.


### Motivation and Context
To reduce the number of deps we have, and address some Github issues
that are related to build ONNX Runtime from source.
In PR #3000 , I added a custom implementation of std::mutex . It was
mainly because at that time std::mutex's default constructor was not
trivial on Windows. If you had such a mutex as a global var, it could
not be initialized at compile time. Then VC++ team fixed this issue.
Therefore we don't need this custom implementation anymore.

This PR also removes nsync. I ran several models tests on Linux. I
didn't see any perf difference.
This PR also reverts PR #21005 , which is no longer needed since conda
has updated its msvc runtime DLL.

This PR unblocks #22173 and resolves #22092 . We have a lot of open
issues with nsync. This PR can resolve all of them.
ishwar-raut1 pushed a commit to ishwar-raut1/onnxruntime that referenced this pull request Nov 19, 2024
### Description
1. Remove the onnxruntime::OrtMutex class and replace it with
~absl::Mutex~ std::mutex.
2. After this change, most source files will not include <Windows.h>
indirectly.


### Motivation and Context
To reduce the number of deps we have, and address some Github issues
that are related to build ONNX Runtime from source.
In PR microsoft#3000 , I added a custom implementation of std::mutex . It was
mainly because at that time std::mutex's default constructor was not
trivial on Windows. If you had such a mutex as a global var, it could
not be initialized at compile time. Then VC++ team fixed this issue.
Therefore we don't need this custom implementation anymore.

This PR also removes nsync. I ran several models tests on Linux. I
didn't see any perf difference.
This PR also reverts PR microsoft#21005 , which is no longer needed since conda
has updated its msvc runtime DLL.

This PR unblocks microsoft#22173 and resolves microsoft#22092 . We have a lot of open
issues with nsync. This PR can resolve all of them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants