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

numactl.c: Refactor print_node_cpus to display CPU ranges #195

Merged
merged 4 commits into from
Jan 6, 2024

Conversation

vishalc-ibm
Copy link
Contributor

@vishalc-ibm vishalc-ibm commented Oct 17, 2023

This commit refines the print_node_cpus function to enhance output readability, especially for systems with numerous CPUs. Initially, the function itemized each CPU, leading to potentially lengthy and cluttered outputs. With this update, the function now identifies and prints contiguous CPU ranges as hyphen-separated sequences (e.g., 1-4) instead of individual lists (e.g., 1 2 3 4), streamlining the display.

Additionally, to provide a quick overview, a count of the total available CPUs is appended at the end of the output, allowing users to instantly grasp the total number without manual counting.

While the output strategy underwent these improvements, the function's core logic, error-handling, and interactions with the NUMA library remain intact, ensuring reliability and consistency with previous behavior.

Signed-off-by: Vishal Chourasia vishalc@linux.ibm.com

…CPUs

This commit enhances the readability and conciseness of the output produced by the print_node_cpus function. Previously, the function listed each available CPU individually, which, especially for systems with a high number of CPUs, resulted in extensive and somewhat cluttered output. 

The key changes include the introduction of logic to identify contiguous ranges of CPUs, thereby allowing these sequences to be printed as hyphen-separated ranges (e.g., 1-4) instead of itemized lists (e.g., 1 2 3 4). This change significantly streamlines the output, making it more digestible and easier to interpret, particularly for systems with large numbers of CPUs.

By tracking the start and end of each CPU range and only printing once a discontinuity (or the end of the list) is encountered, the function can represent the same information more compactly and clearly. This update respects the original structure and logic of the function, altering only the output strategy to improve clarity without affecting the core functionality.

It's worth noting that the function's error-handling and interaction with the NUMA library remain unchanged, preserving the original behavior in scenarios where system queries might fail or return unexpected results.
Copy link
Collaborator

@luochenglcs luochenglcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reduce code cyclomatic complexity.

numactl.c Outdated Show resolved Hide resolved

This commit enhances the readability and conciseness of the output produced by the print_node_cpus function. Previously, the function listed each available CPU individually, which, especially for systems with a high number of CPUs, resulted in extensive and somewhat cluttered output.

The key changes include the introduction of logic to identify contiguous ranges of CPUs, thereby allowing these sequences to be printed as hyphen-separated ranges (e.g., 1-4) instead of itemized lists (e.g., 1 2 3 4). This change significantly streamlines the output, making it more digestible and easier to interpret, particularly for systems with large numbers of CPUs.

By tracking the start and end of each CPU range and only printing once a discontinuity (or the end of the list) is encountered, the function can represent the same information more compactly and clearly. This update respects the original structure and logic of the function, altering only the output strategy to improve clarity without affecting the core functionality.

It's worth noting that the function's error-handling and interaction with the NUMA library remain unchanged, preserving the original behavior in scenarios where system queries might fail or return unexpected results.

Signed-off-by: Vishal Chourasia vishalc@linux.ibm.com
Copy link
Collaborator

@luochenglcs luochenglcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good to me.


This commit enhances the readability and conciseness of the output produced by the print_node_cpus function. Previously, the function listed each available CPU individually, which, especially for systems with a high number of CPUs, resulted in extensive and somewhat cluttered output.

The key changes include the introduction of logic to identify contiguous ranges of CPUs, thereby allowing these sequences to be printed as hyphen-separated ranges (e.g., 1-4) instead of itemized lists (e.g., 1 2 3 4). This change significantly streamlines the output, making it more digestible and easier to interpret, particularly for systems with large numbers of CPUs.

By tracking the start and end of each CPU range and only printing once a discontinuity (or the end of the list) is encountered, the function can represent the same information more compactly and clearly. This update respects the original structure and logic of the function, altering only the output strategy to improve clarity without affecting the core functionality.

It's worth noting that the function's error-handling and interaction with the NUMA library remain unchanged, preserving the original behavior in scenarios where system queries might fail or return unexpected results.

Signed-off-by: Vishal Chourasia vishalc@linux.ibm.com
@vishalc-ibm
Copy link
Contributor Author

vishalc-ibm commented Oct 25, 2023

I have added code to print number of CPUs as well.

node 1 cpus: 80-103 (24)

@luochenglcs
Copy link
Collaborator

Annotations are too redundant, and obvious annotations can be removed.

This commit refines the print_node_cpus function to enhance output readability, especially for systems with numerous CPUs. Initially, the function itemized each CPU, leading to potentially lengthy and cluttered outputs. With this update, the function now identifies and prints contiguous CPU ranges as hyphen-separated sequences (e.g., 1-4) instead of individual lists (e.g., 1 2 3 4), streamlining the display.

Additionally, to provide a quick overview, a count of the total available CPUs is appended at the end of the output, allowing users to instantly grasp the total number without manual counting.

While the output strategy underwent these improvements, the function's core logic, error-handling, and interactions with the NUMA library remain intact, ensuring reliability and consistency with previous behavior.

Signed-off-by: Vishal Chourasia vishalc@linux.ibm.com
@vishalc-ibm
Copy link
Contributor Author

vishalc-ibm commented Nov 28, 2023

Annotations are too redundant, and obvious annotations can be removed.

I agree.

@vishalc-ibm
Copy link
Contributor Author

Hi @luochenglcs Can this PR be merged?

@luochenglcs
Copy link
Collaborator

Hi @luochenglcs Can this PR be merged?

Sorry, i currently have committer permissions, but don't have write permissions, so I can't merge.

@andikleen andikleen merged commit a68578c into numactl:master Jan 6, 2024
@andikleen
Copy link
Contributor

Thanks!

@andikleen
Copy link
Contributor

I had to change the code to only be active with a flag. Otherwise test/checktopology was broken. There might be other people using scripts parsing this output so we cannot change the output format without an opt-in. Sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants