Macchina-CLI / libmacchina

A library providing access to all sorts of system information.
https://crates.io/crates/libmacchina
MIT License
68 stars 20 forks source link

Windows: Implement `cpu_physical_cores` and `cpu_cores` #145

Open Carterpersall opened 1 year ago

Carterpersall commented 1 year ago

Hello, I'm a contributor to https://github.com/lptstr/winfetch (you've talked to the maintainer in #112), and would like to start applying some of the experience I gained there to this project.

CarterLi commented 1 year ago

You may get physical cores and logical cores ( threads ) at the same time with GetLogicalProcessorInformationEx(RelationAll)

https://github.com/LinusDierheimer/fastfetch/blob/b3da6b0e89c0decb9ea648e1d98a75fa6ac40225/src/detection/cpu/cpu_windows.c#L17

Because this method requires copying more data from the kernel, I'm not sure whether it's faster than your implementation. Just another idea.

Carterpersall commented 1 year ago

I added several alternative implementations

Implementation Speeds:

Plugged in:

On Battery life:

When heavily throttled (used power plans to downclock CPU to 1.06 GHz to simulate a slow computer)

grtcdr commented 1 year ago

There's virtually no difference between some of the implementations and those of num_cpus, why not call those two functions directly?

CarterLi commented 1 year ago

According to stackoverflow, WMI, registry and GetLogicalProcessorInformationEx are only reliable ways to get the number of cpu logical cores (threads)

Carterpersall commented 1 year ago

According to stackoverflow, WMI, registry and GetLogicalProcessorInformationEx are only reliable ways to get the number of cpu logical cores (threads)

That does seem to be the case. I've made a commit that addresses it. cpu_cores now uses GetLogicalProcessorInformationEx to get the number of cores, and has Registry and WMI based implementations as backups. cpu_physical_cores now uses GetLogicalProcessorInformationEx as well.

I used the implementation here as the basis of my implementation of GetLogicalProcessorInformationEx.

IAMSolaara commented 2 months ago

Hi, I'm making a tool that can benefit from this. Any way we can get this merged?

grtcdr commented 2 months ago

I've got a Windows VM lying around but I'm a bit busy at the moment. If this works correctly, I don't have a problem accepting the PR. I would appreciate @uttarayan21's input on the state of this PR because he's much more knowledgeable on Windows than I am.