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.

Sign up for GitHub

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

Jump to bottom

Cache a global static variable initialized with std::locale::classic() to avoid scalability issues associated with std::locale::classic. #4443

Closed
LarryOsterman opened this issue Mar 13, 2023 · 0 comments · Fixed by #4448
Closed

Cache a global static variable initialized with std::locale::classic() to avoid scalability issues associated with std::locale::classic. #4443

LarryOsterman opened this issue Mar 13, 2023 · 0 comments · Fixed by #4448
Assignees
@antkmsft
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library.

Comments

@LarryOsterman
Copy link
Member

The storage team reported the following issue:

We recently deployed a build with Azure-Storage-Blobs_12.6.2, but we noticed that there were high CPU usage within the SDK.

After some investigation, we found that this might be caused by: : Unnecessary locking around classic locale · Issue #3030 · microsoft/STL (github.com). We are currently using “set(VCPKG_PLATFORM_TOOLSET v142)” in order to build the SDK, which corresponds to the Visual Studio 2019 platform toolset. A short term mitigation would be for us to try using an older azure-core-cpp version before this change was introduced using vcpkg, but do you all have any recommendations for what the best course of action is?

The problem was introduced in a recent change to modify calls to std::toupper and a number of other functions to include a call to std::locale::classic() to select the classic "C" locale. The problem is that as called out in the linked issue above, there is a scalability problem with this API.

Instead of calling std::locale::classic() in the inner loop, it appears that the value of std::locale::classic() can be safely cached locally thus avoiding the scalability problems.
@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Mar 13, 2023
@antkmsft antkmsft mentioned this issue Mar 15, 2023
Merged
@RickWinter RickWinter added bug This issue requires a change to an existing behavior in the product in order to be resolved. Azure.Core Client This issue points to a problem in the data-plane of the library. labels Mar 20, 2023
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Mar 20, 2023
azure-sdk added a commit to azure-sdk/vcpkg that referenced this issue Apr 6, 2023
@azure-sdk
[azure-core-cpp] Update to 1.8.1
573e20c 
## 1.8.1 (2023-04-06)

### Bugs Fixed

- [[microsoft#4213]](Azure/azure-sdk-for-cpp#4213) Fixed a bug where `Host` request header is not set for non-default port (80, 443).
- [[microsoft#4443]](Azure/azure-sdk-for-cpp#4443) Fixed potentially high CPU usage on Windows.

### Other Changes

- Libcurl transport doesn't add `Content-Length` request header for GET/HEAD/DELETE requests anymore.
vicroms pushed a commit to microsoft/vcpkg that referenced this issue Apr 10, 2023
@azure-sdk
[azure-core-cpp] Update to 1.8.1 (#30703)
34221eb 
## 1.8.1 (2023-04-06)

### Bugs Fixed

- [[#4213]](Azure/azure-sdk-for-cpp#4213) Fixed a bug where `Host` request header is not set for non-default port (80, 443).
- [[#4443]](Azure/azure-sdk-for-cpp#4443) Fixed potentially high CPU usage on Windows.

### Other Changes

- Libcurl transport doesn't add `Content-Length` request header for GET/HEAD/DELETE requests anymore.
azure-sdk added a commit to azure-sdk/vcpkg that referenced this issue May 4, 2023
@azure-sdk
[azure-identity-cpp] Update to 1.5.0
b487b7e 
## 1.5.0 (2023-05-04)

### Features Added

- Added support for challenge-based and multi-tenant authentication.
- Added `DefaultAzureCredential`.

### Bugs Fixed

- [[microsoft#4443]](Azure/azure-sdk-for-cpp#4443) Fixed potentially high CPU usage on Windows.

### Other Changes

- Improved diagnostics to utilize `Azure::Core::Credentials::TokenCredential::GetCredentialName()`.
- Improved log messages.
@antkmsft antkmsft mentioned this issue May 4, 2023
Merged
BillyONeal pushed a commit to microsoft/vcpkg that referenced this issue May 4, 2023
@azure-sdk @antkmsft
[azure-core-cpp] Update to May Release for Core Packages (#31256)
60e2c07 
* [azure-security-keyvault-administration-cpp] Update to 4.0.0-beta.3

* [azure-identity-cpp] Update to 1.5.0
## 1.5.0 (2023-05-04)

### Features Added

- Added support for challenge-based and multi-tenant authentication.
- Added `DefaultAzureCredential`.

### Bugs Fixed

- [[#4443]](Azure/azure-sdk-for-cpp#4443) Fixed potentially high CPU usage on Windows.

### Other Changes

- Improved diagnostics to utilize `Azure::Core::Credentials::TokenCredential::GetCredentialName()`.
- Improved log messages.

* [azure-core-cpp] Update to 1.9.0
## 1.9.0 (2023-05-04)

### Features Added

- Added the ability to ignore invalid certificate common name for TLS connections in WinHTTP transport.
- Added `DisableTlsCertificateValidation` in `TransportOptions`.
- Added `TokenCredential::GetCredentialName()` to be utilized in diagnostic messages. If you have any custom implementations of `TokenCredential`, it is recommended to pass the name of your credential to `TokenCredential` constructor. The old parameterless constructor is deprecated.
- Added support for challenge-based and multi-tenant authentication.

### Bugs Fixed

- Fixed the UUID generation so the variant is RFC 4122 conforming.

### Other Changes

- [[#4352]](Azure/azure-sdk-for-cpp#4352) Fixed compilation error on Visual Studio 2017. (A community contribution, courtesy of _[jorgen](https://github.com/jorgen)_)

### Acknowledgments

Thank you to our developer community members who helped to make Azure Core better with their contributions to this release:

- Jorgen Lind _([GitHub](https://github.com/jorgen))_

---------

Co-authored-by: Anton Kolesnyk <[email protected]>
Co-authored-by: Anton Kolesnyk <[email protected]>
@github-actions github-actions bot locked and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@antkmsft antkmsft

Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix potentially high CPU usage on Windows antkmsft/azure-sdk-for-cpp
3 participants
@RickWinter @LarryOsterman @antkmsft