HorlogeSkynet / archey4

:computer: Maintained fork of the original Archey (Linux) system tool
https://git.io/archey4
GNU General Public License v3.0
295 stars 37 forks source link

[CPU] [FEATURE/BREAKING] Multiple CPUs support #90

Closed HorlogeSkynet closed 4 years ago

HorlogeSkynet commented 4 years ago

Description

This is an API-breaking change. Now multiple CPUs should be supported, and number of cores per CPU will be shown by default.

Below examples show output changes for a dual CPUs setup (respectively with N & M cores).

Text output change :

- CPU: A-CPU-MODEL-NAME_1
+ CPU: N x A-CPU-MODEL-NAME_1, M x A-CPU-MODEL-NAME_2

JSON API change :

- "CPU": "A-CPU-MODEL-NAME_1",
+ "CPU": [
+     {"A-CPU-MODEL-NAME_1": N},
+     {"A-CPU-MODEL-NAME_2": M}
+ ],

See also new one_line & show_cores options available for CPU entry type.

Reason and / or context

It appears Archey does not support multiple CPUs (only the first one would be displayed). Additionally, adding the number of cores may be relevant.

See also https://github.com/dylanaraps/neofetch/issues/1467 partially related. (@Nalorokk's output suggestion has been kindly implemented there.)

How has this been tested ?

Locally and unit tests.

Types of changes :

Checklist :

Nalorokk commented 4 years ago

Seems like detection may be broken in some cases

https://gist.github.com/Nalorokk/91f4cced6f61ad72b6d37595c2662256

HorlogeSkynet commented 4 years ago

@Nalorokk Oh you actually tried this out 😅 I've thought about your case since this PR is opened, and yeah, when all different CPUs have the same name, cores are just being added (imitating Neofetch current behavior I guess then). Would you mind sending me your /proc/cpuinfo content (as long as lscpu output if you got it installed) so I may work on this (I actually don't have such a setup available) ?

Bye, thanks 👋

Nalorokk commented 4 years ago

@HorlogeSkynet here it is https://gist.github.com/Nalorokk/5ce83329610bcc5102639230428014c6 . There is two examples from different machines

HorlogeSkynet commented 4 years ago

Thank you very much for this. I've come up with an algorithm, which would give you below final Python objects (if I manage to implement it) :

# For your 1st machine :
[
    {'Intel(R) Xeon(R) CPU E5450 @ 3.00GHz': 4},
    {'Intel(R) Xeon(R) CPU E5450 @ 3.00GHz': 4}
]

# For your second machine :
[
    {'Intel(R) Xeon(R) CPU E5620 @ 2.40GHz': 8},
    {'Intel(R) Xeon(R) CPU E5620 @ 2.40GHz': 8}
]

If you're OK with the idea, I'll update this PR with a new proposal/implementation.

HorlogeSkynet commented 4 years ago

Hey @Nalorokk, please find on this branch a more robust implementation that should support many setups. Bye, waiting for your feedback before merging this 👋

Nalorokk commented 4 years ago

Hello @HorlogeSkynet . I tried new branch and got such output

#########.   .########`      CPU: 4 x Intel(R) Xeon(R) CPU E5450 @ 3.00GHz, 4 x Intel(R) Xeon(R) CPU E5450 @ 3.00GHz

It is way better than before. Maybe line is kinda too long though. But I just thought about it a little more and there is some another problems: 1) There is quad cpu motherboards 2) Some of them may work with just 3 cpus installed. 3) Some motherboards might work with different cpu's mixed.

I don't have quad cpu setups around, but maybe will try mixing different cpus in one motherboard, it happens that i have combo that might work together

HorlogeSkynet commented 4 years ago

It is way better than before. Maybe line is kinda too long though.

Yes, there is a new option called one_line, for CPU entry. If you disable it, each entry will be on their respective line. Maybe this option should be disabled be default ? What do you think about that ?

But I just thought about it a little more and there is some another problems:

1. There is quad cpu motherboards
2. Some of them may work with just 3 cpus installed.
3. Some motherboards might work with different cpu's mixed.

Latest implementation should correctly deal with such setups.


Thank you very much again for your feedback @Nalorokk. 👋

Nalorokk commented 4 years ago

@HorlogeSkynet i think it would be better with one line disabled by default. Thank you for your work on this topic 👍

HorlogeSkynet commented 4 years ago

i think it would be better with one line disabled by default.

Fair enough, I've just got it disabled by default for CPU (as long as GPU, for configuration consistency purposes).

Thank you for your work on this topic 👍

Sure, thank you for your fast responses and your time @Nalorokk. Cheers 🍻