Refactor MemoryProfiler #356
Open
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
Refactored the MemoryProfiler code replacing C file I/O and string parsing with C++ streams and regex. Look for environment variables only once. Signed-off-by: Christoph Niethammer <[email protected]>
HomesGH
requested changes
Dec 12, 2024
There was a problem hiding this comment.
Nice effort to modernize the plugin!
Just some comments:
- Checks if files exist/error handling of input
- More comments in the code would be beneficial
- Test not only with unit tests but also run a real simulation and compare the result with the previous implementation
- Keep in mind that
std::regexmight be slower compared to the previous solution
| #include "sys/sysinfo.h" | ||
|
|
||
| #ifndef _SX |
There was a problem hiding this comment.
What is the preprocessor macro _SX? I cannot find any definition in our code base or online.
There was a problem hiding this comment.
This preprocessor macro is specific to NEC platforms which do not run standard unix/linux like the https://en.wikipedia.org/wiki/NEC_SX-Aurora_TSUBASA.
There was a problem hiding this comment.
Thank you for the clarification 👍 Does anybody still use ls1 on those machines or is it just legacy?
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.
Refactored the MemoryProfiler code replacing C file I/O and string parsing with C++ streams and regex. Look for environment variables only once.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.