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

CPU Temp Shows 0.0 | 2023 14", M2 Max (12 Core), Sonoma 14.5 #32

Closed
kyleyannelli opened this issue Aug 12, 2024 · 17 comments
Closed

CPU Temp Shows 0.0 | 2023 14", M2 Max (12 Core), Sonoma 14.5 #32

kyleyannelli opened this issue Aug 12, 2024 · 17 comments

Comments

@kyleyannelli
Copy link
Contributor

kyleyannelli commented Aug 12, 2024

╭─kyannelli::mbp @ ~/smctemp
╰─ » smctemp -c
0.0
╭─kyannelli::mbp @ ~/smctemp
╰─ » smctemp -g
40.3
Screenshot 2024-08-11 at 8 10 58 PM
Model Name:	MacBook Pro
  Model Identifier:	Mac14,5
  Model Number:	Z17J000AKLL/A
  Chip:	Apple M2 Max
  Total Number of Cores:	12 (8 performance and 4 efficiency)
  Memory:	32 GB
  System Firmware Version:	10151.121.1
  OS Loader Version:	10151.121.1

It seems the prior support added for the M2 Max 8 core PR #14 does not work for the 12 core M2 Max.

@kyleyannelli
Copy link
Contributor Author

kyleyannelli commented Aug 12, 2024

It looks like the sensor values have changed https://github.com/exelban/stats/blob/e3fb0ae9b369efa33b4e5708a39abc95661cde11/Modules/Sensors/values.swift#L373-L389

Update

I was going to make a PR as I added the keys and added simple core count logic. However, I can only get proper values to read upon right after building. Not sure what's going on here. I'm not familiar with C++ or MacOS at this level.

╭─kyannelli::mbp @ ~/smctemp
╰─ » sudo make clean && sudo rm -rf /usr/local/bin/smctemp && sudo make install && smctemp -c
rm -f -r smctemp smctemp.o smctemp_string.o smctemp.dSYM libsmctemp.a
g++ -Wall -std=c++17 -g -framework IOKit -DARCH_TYPE_ARM64 -o smctemp.o -c smctemp.cc
clang: warning: -framework IOKit: 'linker' input unused [-Wunused-command-line-argument]
smctemp.cc:68:1: warning: 'OSSpinLock' is deprecated: first deprecated in macOS 10.12 - Use os_unfair_lock() from <os/lock.h> instead [-Wdeprecated-declarations]
OSSpinLock g_keyInfoSpinLock = 0;
^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/libkern/OSSpinLockDeprecated.h:79:17: note: 'OSSpinLock' has been explicitly marked deprecated here
typedef int32_t OSSpinLock OSSPINLOCK_DEPRECATED_REPLACE_WITH(os_unfair_lock);
                ^
smctemp.cc:160:3: warning: 'IOMasterPort' is deprecated: first deprecated in macOS 12.0 [-Wdeprecated-declarations]
  IOMasterPort(MACH_PORT_NULL, &masterPort);
  ^~~~~~~~~~~~
  kIOMainPortDefault
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/IOKit.framework/Headers/IOKitLib.h:143:1: note: 'IOMasterPort' has been explicitly marked deprecated here
IOMasterPort( mach_port_t       bootstrapPort,
^
smctemp.cc:221:3: warning: 'OSSpinLockLock' is deprecated: first deprecated in macOS 10.12 - Use os_unfair_lock_lock() from <os/lock.h> instead [-Wdeprecated-declarations]
  OSSpinLockLock(&g_keyInfoSpinLock);
  ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/libkern/OSSpinLockDeprecated.h:99:9: note: 'OSSpinLockLock' has been explicitly marked deprecated here
void    OSSpinLockLock( volatile OSSpinLock *__lock );
        ^
smctemp.cc:249:3: warning: 'OSSpinLockUnlock' is deprecated: first deprecated in macOS 10.12 - Use os_unfair_lock_unlock() from <os/lock.h> instead [-Wdeprecated-declarations]
  OSSpinLockUnlock(&g_keyInfoSpinLock);
  ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/libkern/OSSpinLockDeprecated.h:105:9: note: 'OSSpinLockUnlock' has been explicitly marked deprecated here
void    OSSpinLockUnlock( volatile OSSpinLock *__lock );
        ^
smctemp.cc:333:38: warning: format string is not a string literal (potentially insecure) [-Wformat-security]
  snprintf(val.key, sizeof(val.key), key);
                                     ^~~
smctemp.cc:333:38: note: treat the string as an argument to avoid this
  snprintf(val.key, sizeof(val.key), key);
                                     ^
                                     "%s",
5 warnings generated.
g++ -Wall -std=c++17 -g -framework IOKit -DARCH_TYPE_ARM64 -o smctemp_string.o -c smctemp_string.cc
clang: warning: -framework IOKit: 'linker' input unused [-Wunused-command-line-argument]
g++ -Wall -std=c++17 -g -framework IOKit -DARCH_TYPE_ARM64 -o smctemp smctemp.o smctemp_string.o main.cc
install -d /usr/local/bin
install -m 0755 smctemp /usr/local/bin
Sensor Value: 41.4906
Sensor Value: 39.3315
Sensor Value: 41.0069
Sensor Value: 39.7614
Sensor Value: 41.5559
Sensor Value: 39.2271
Sensor Value: 42.0472
Sensor Value: 39.2348
Sensor Value: 41.1313
Sensor Value: 38.4611
Sensor Value: 41.1007
Sensor Value: 39.113
40.3
╭─kyannelli::mbp @ ~/smctemp
╰─ » smctemp -c
Sensor Value: 6.7
Sensor Value: 4.633
Sensor Value: 6.7
Sensor Value: 4.633
Sensor Value: 6.7
Sensor Value: 4.633
Sensor Value: 6.7
Sensor Value: 4.633
Sensor Value: 6.7
Sensor Value: 4.633
Sensor Value: 6.7
Sensor Value: 4.633
0.0

@narugit
Copy link
Owner

narugit commented Aug 12, 2024

@kyleyannelli
Could you give me the result of below command?

$ system_profiler SPHardwareDataType | grep -iv ID
$ smctemp -l

@kyleyannelli
Copy link
Contributor Author

Hi @narugit . You have the output of system_profiler above in my initial issue message.
I've attached the smctemp -l output as a text file.
smctemp_dash_l.out.txt

@narugit
Copy link
Owner

narugit commented Aug 12, 2024

Hi @kyleyannelli
I'm sorry, I missed that. Thank you for the attachment.

I've reviewed the output from smctemp -l.
Could you take a screenshot of the Sensors app at the moment you run the smctemp -l command?
This way, we can compare the information from both the command-line tool and the GUI app, which will help us with a more accurate analysis.

@kyleyannelli
Copy link
Contributor Author

No need to apologize!

Here's the screenshot and output.
screenshot_stats_smctemp_dash_l.out.txt
Screenshot 2024-08-12 at 8 06 04 PM

@narugit
Copy link
Owner

narugit commented Aug 13, 2024

@kyleyannelli
Thank you!
I've just remembered that the M2 sensors are a bit strange.

#14 (comment)

It'll return like "36.1" sometimes, then 4.2..

Therefore, we've added the attempt logic to read correct sensor value by adding -n option (attempt times).
How about running below command?
$ smctemp -c -n10

@kyleyannelli
Copy link
Contributor Author

No such luck. I have a better chance of getting a real result with higher values such as n50. However, this makes the runtime for the command much too long. Still with n50 it seems to be a low chance of getting a proper value.

@kyleyannelli
Copy link
Contributor Author

kyleyannelli commented Aug 13, 2024

I also want to reiterate I almost always get a proper value if I run smctemp -c after installing or building with &&. Even if I build and run the command manually, it does not show this behavior. Only if it is in conjunction via &&.
Ex. sudo make clean && sudo make && ./smctemp -c

@narugit
Copy link
Owner

narugit commented Aug 13, 2024

@kyleyannelli

No such luck. I have a better chance of getting a real result with higher values such as n50. However, this makes the runtime for the command much too long. Still with n50 it seems to be a low chance of getting a proper value.

I see.
The stats app does the same thing, getting sensor values at regular intervals and updating only when it gets values in the normal range (10°C~120°C).
https://github.com/exelban/stats/blob/e3fb0ae9b369efa33b4e5708a39abc95661cde11/Modules/Sensors/readers.swift#L124-L125

I guess smctemp is not an app that keeps running in the background, so it might not be a good match.
How about decrease the sleep value of

usleep(1'000'000);
and increase the number of attempts?
I don't have M2 Max on hand, so I hope you can find a good value.

@narugit
Copy link
Owner

narugit commented Aug 13, 2024

@kyleyannelli

Even if I build and run the command manually, it does not show this behavior.

Do you mean your manual command works properly?

@kyleyannelli
Copy link
Contributor Author

kyleyannelli commented Aug 13, 2024

Clarification

@kyleyannelli

Even if I build and run the command manually, it does not show this behavior.

Do you mean your manual command works properly?

Sorry let me demonstrate that better.
Running this (separate commands)

$ sudo make clean
$ sudo make
$ ./smctemp -c

Will almost always produce incorrect values.

While

$ sudo make clean && sudo make && ./smctemp -c

Will almost always give me proper values. This doesn't make sense to me as it's compiled. I'm not really sure why the behavior is displaying like this.

Sleep Time Adjustment

I've set the sleep to 100ms and run with -n500. Obviously this consumes a bit more CPU, but nothing of note.

I made a test script to trial how many times the output was 0.0 and over 1000 runs.
With 100ms sleep and -n500 it is successful 80% of the time. With the median command run time being ~1 second. Worst case 11 seconds.

This will have to be good enough as smctemp is the only CLI tool I have.
I can finally display my CPU temp in my tmux status bar with this 😃.

Pull Request?

Edit: PR would only include the 4 extra sensors for 12 - core M2. Not my sleep adjustment.
Since I have the m2 12 core efficiency sensors on my branch do you want me to make a PR so you can add them?
I realized the core count checking logic I added might not matter as invalid values get dropped. So should I adjust it to add all the sensors in m2 regardless of this (eg remove getLogicalCPUCount)? Below is a snippet.

  int getLogicalCPUCount() {
      int logicalCPUCount = 0;
      size_t bufferLength = sizeof(logicalCPUCount);
      sysctlbyname("hw.logicalcpu", &logicalCPUCount, &bufferLength, nullptr, 0);
      return logicalCPUCount;
  }
  // existing code

  const std::string cpumodel = getCPUModel();
  const int logicalcpucount = getLogicalCPUCount();
  if (cpumodel.find("m3") != std::string::npos) {  // Apple M3
  // exisiting code
  } else if (cpumodel.find("m2") != std::string::npos) {  // Apple M2 8 core
    if (logicalcpucount == 12) { // Apple M2 12 Core
      // Add eff cores
      sensors.emplace_back(static_cast<std::string>(kSensorTp1h));
      sensors.emplace_back(static_cast<std::string>(kSensorTp1t));
      sensors.emplace_back(static_cast<std::string>(kSensorTp1p));
      sensors.emplace_back(static_cast<std::string>(kSensorTp1l));
      // Rest of cores are performance
   }
    // CPU core 1
    sensors.emplace_back(static_cast<std::string>(kSensorTp01));
    // CPU core 2
    sensors.emplace_back(static_cast<std::string>(kSensorTp09));
    // CPU core 3
    sensors.emplace_back(static_cast<std::string>(kSensorTp0f));
    // CPU core 4
    sensors.emplace_back(static_cast<std::string>(kSensorTp0n));
    // CPU core 5
    sensors.emplace_back(static_cast<std::string>(kSensorTp05));
    // CPU core 6
    sensors.emplace_back(static_cast<std::string>(kSensorTp0D));
    // CPU core 7
    sensors.emplace_back(static_cast<std::string>(kSensorTp0j));
    // CPU core 8
    sensors.emplace_back(static_cast<std::string>(kSensorTp0r));

Sorry for such a long winded reply. I just wanted to be thorough. Hope it's not too much.

@narugit
Copy link
Owner

narugit commented Aug 13, 2024

@kyleyannelli
It's puzzling that there’s a difference in the results when using && versus not using it..

The results of your sleep time and retry tuning tests are quite interesting.
It's surprising that even with a high number of retries, you can still get abnormal values.
Perhaps we should consider adding hyper attempt feature with -n set to 0, which could lead to an infinite loop, or try with a very large number of retries.

Regarding the pull request, please go ahead.
As you mentioned, since invalid sensor values are skipped, it seems that the getLogicalCPUCount mechanism might not be necessary.

@kyleyannelli
Copy link
Contributor Author

@narugit Yes, I'm a bit puzzled by the difference in results via && as well. It is really disappointing Apple doesn't give us a good interface for something seemingly trivial.

I can play around with the -n0 idea later tomorrow. For now, I've made the pull request.

Thank you for your quick responses and willingness to try things out.

@kyleyannelli
Copy link
Contributor Author

@narugit I've played around a bit with the hyper attempt feature. It seems a sleep value of 25ms and 1000 retries will consistently get a value. A sleep value any lower will yield more failure than success.

Testing

Running the command as such time for i in {1..10000}; do ./smctemp -c -n1000; done then dividing the resultant seconds by 10000 to derive the average command time. Using this method I found that it took an average of 0.011 seconds to run the ./smctemp -c -n1000 command. I ran this trial 2 times and found that the command did not fail once. Therefore, I'm not sure an infinite number of retries would be necessary.

What do you think?

@narugit
Copy link
Owner

narugit commented Aug 14, 2024

@kyleyannelli
Thank you for sharing the test results.
Indeed, looking at your case, it seems that setting an appropriate interval is more effective for obtaining stable sensor values than simply increasing the number of trials.
I wonder if there were any cases where the maximum time took a few seconds out of the 10,000 trials?

Since the optimal interval likely depends on hardware factors as well, I think it's a good idea to add option to set the interval within a realistic range (20ms to 1000ms).

@narugit
Copy link
Owner

narugit commented Aug 14, 2024

Since the optimal interval likely depends on hardware factors as well, I think it's a good idea to add option to set the interval within a realistic range (20ms to 1000ms).

I've created the PR #34

@narugit
Copy link
Owner

narugit commented Aug 19, 2024

Hi @kyleyannelli
I've merged the PRs below, so I'll be closing this issue.

Thank you for your help with opening the issue and debugging!

As I'm preparing to release via Homebrew, I need some more stars. (#3 (comment))
If you don't mind, giving star to this repository would be greatly appreciated!

@narugit narugit closed this as completed Aug 19, 2024
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

No branches or pull requests

2 participants