Fix omrsysinfo_get_memory_info for non-english locales on windows #7566
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The windows implementation of omrsysinfo_get_memory_info used PdhAddCounter which requires the counter path string to be localized. Since we weren't doing so the function failed on non-english windows systems. Replacing the call with the language neutral PdhAddEnglishCounter fixes the problem. I added some debug to check and print the error code. Before the change: ``` omrsysinfo_get_memory_info: after GlobalMemoryStatusEx 4200 omrsysinfo_get_memory_info: after PdhOpenQuery 189500 omrsysinfo_get_memory_info: after PdhAddCounter 1 532003600 PdhAddCounter FAILED with c0000bb8 ``` After the change: ``` omrsysinfo_get_memory_info: after GlobalMemoryStatusEx 5000 omrsysinfo_get_memory_info: after PdhOpenQuery 108600 omrsysinfo_get_memory_info: after PdhAddCounter 1 689300300 omrsysinfo_get_memory_info: after PdhAddCounter 2 309400 omrsysinfo_get_memory_info: after PdhAddCounter 3 161600 omrsysinfo_get_memory_info: after PdhCollectQueryData 201400 totalSwap 46477017088 ```
|
@babsingh : please review |
|
Commit guidelines: https://github.com/eclipse-omr/omr/blob/master/CONTRIBUTING.md#commit-guidelines
|
babsingh
reviewed
Dec 3, 2024
Co-authored-by: Babneet Singh <[email protected]>
Co-authored-by: Babneet Singh <[email protected]>
babsingh
reviewed
Dec 3, 2024
There was a problem hiding this comment.
The commits will need to be squashed. The primary commit will need to be updated as per #7566 (comment).
| if (ERROR_SUCCESS != status) { | ||
| Trc_PRT_sysinfo_get_memory_info_failedAddingCounter("Commit Limit", status); | ||
| Trc_PRT_sysinfo_get_memory_info_Exit(OMRPORT_ERROR_SYSINFO_ERROR_READING_MEMORY_INFO); | ||
| PdhCloseQuery(statsHandle); | ||
| return OMRPORT_ERROR_SYSINFO_ERROR_READING_MEMORY_INFO; | ||
| } | ||
|
|
||
| status = PdhAddCounter(statsHandle, | ||
| status = PdhAddEnglishCounter(statsHandle, |
There was a problem hiding this comment.
#7566 (comment) also needs to be applied here.
| @@ -951,7 +953,7 @@ omrsysinfo_get_memory_info(struct OMRPortLibrary *portLibrary, struct J9MemoryIn | |||
| return OMRPORT_ERROR_SYSINFO_ERROR_READING_MEMORY_INFO; | |||
| } | |||
|
|
|||
| status = PdhAddCounter(statsHandle, | |||
| status = PdhAddEnglishCounter(statsHandle, | |||
There was a problem hiding this comment.
#7566 (comment) also needs to be applied here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The windows implementation of omrsysinfo_get_memory_info used PdhAddCounter which requires the counter path string to be localized. Since we weren't doing so the function failed on non-english windows systems. Replacing the call with the language neutral PdhAddEnglishCounter fixes the problem.
I added some debug to check and print the error code, and tested by setting Windows to the French locale.
Before the change:
After the change:
Doc for PdhAddEnglishCounter: https://learn.microsoft.com/en-us/windows/win32/api/pdh/nf-pdh-pdhaddenglishcountera