osx CPU freq on apple silicon

[joske]

on Apple Silicon, the hw.cpufreq sysctl doesn't return anything. I found this code in a PR for htop. It gave me some headaches, as it's again using an undocumented API, and uses a strange objective C block syntax which g++ doesn't like at all. I fixed that by compiling that single file as C, and linking it with the rest separately. Not sure if the Makefile hack works on older macs yet.

Another potential issue is that the license for htop is GPL, and btop is apache licensed. Let me know how to handle this.

I did not touch cmake as I don't know it at all (nor do I want to 😁).

But finally, this works, and I can show the highest CPU frequency.

[joske]

Hmm, CI is building it with gcc 12, and that just fails miserably. It doesn't understand the objective C syntax apparently.

On my system, it's compiled with clang 15 without issue...

Why does apple keep all the nice APIs for themselves? 😞 It was the same with the thermal sensors.

[joske]

@aristocratos why are we building in CI with GCC? It seems like this is not going to work at all for this PR.

[imwints]

I did not touch cmake

Not tested, but from a quick look over the commit this seems to be the necessary change

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3f8c546..c887149 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -14,7 +14,7 @@ project("btop"
   VERSION 1.2.13
   DESCRIPTION "A monitor of resources"
   HOMEPAGE_URL "https://github.com/aristocratos/btop"
-  LANGUAGES CXX
+  LANGUAGES C CXX
 )

 include(CheckCXXCompilerFlag)
@@ -61,7 +61,7 @@ add_executable(btop
 )

 if(APPLE)
-  target_sources(btop PRIVATE src/osx/btop_collect.cpp src/osx/sensors.cpp src/osx/smc.cpp)
+  target_sources(btop PRIVATE src/osx/btop_collect.cpp src/osx/CpuFreq.c src/osx/sensors.cpp src/osx/smc.cpp)
 elseif(CMAKE_SYSTEM_NAME STREQUAL "FreeBSD")
   target_sources(btop PRIVATE src/freebsd/btop_collect.cpp)
 elseif(LINUX)
@@ -165,6 +165,7 @@ endif()
 if(APPLE)
   target_link_libraries(btop
     $<LINK_LIBRARY:FRAMEWORK,CoreFoundation> $<LINK_LIBRARY:FRAMEWORK,IOKit>
+    IOReport
   )
 elseif(CMAKE_SYSTEM_NAME STREQUAL "FreeBSD")
   # Avoid version mismatch for libstdc++ when a specific version of GCC is installed and not the
diff --git a/.github/workflows/cmake-macos.yml b/.github/workflows/cmake-macos.yml
index 32d6f7f..c2cb8c5 100644
--- a/.github/workflows/cmake-macos.yml
+++ b/.github/workflows/cmake-macos.yml
@@ -37,6 +37,7 @@ jobs:
       - name: Configure
         run: |
           export LLVM_PREFIX="$(brew --prefix llvm)"
+          export CC="$LLVM_PREFIX/bin/clang"
           export CXX="$LLVM_PREFIX/bin/clang++"
           export CPPFLAGS="-I$LLVM_PREFIX/include"
           export LDFLAGS="-L$LLVM_PREFIX/lib -L$LLVM_PREFIX/lib/c++ -Wl,-rpath,$LLVM_PREFIX/lib/c++ -fuse-ld=$LLVM_PREFIX/bin/ld64.lld"

[joske]

I've made the cpufreq code dependent on the architecture, as it's only needed on aarch64. This satisfies CI.

Also fixed cmake build thanks to @imwints.

Only issue left is the licensing.

[joske]

Well, on x86, the hw.cpufrequency ssyctl returns a value, but it's always the same (probably base freq or so). I think we could also use the same code on x86, but then we NEED to compile macos versions with clang.

[aristocratos]

@joske GPL and Apache is not compatible. GPL requires the whole of the code to be GPL. Did you copy code from htop? Or from a yet merged PR?

In case of the latter, you could ask the author of the PR if he would be ok with you using his code in another project with a different license.

Otherwise it would need to be written from scratch, (it can be inspired by the htop code), but not a exact copy.

why are we building in CI with GCC?

Well, until pretty recently, Clang couldn't build btop at all because of incomplete C++20 support.

Rather than giving up GCC support for MacOS, the PR should be adapted with conditionals for __clang__ (together with the already added __aarch64__) in the source and exclude the objective C code in the Makefile if not compiling with clang.

The default compiler for new Mac Os versions will however most likely be a supported Clang version, so shouldn't be that much of an issue. We just need to add a note in the compile instructions for Mac Os that Clang is needed for Cpu frequency on Apple Silicon.

Well, on x86, the hw.cpufrequency ssyctl returns a value, but it's always the same (probably base freq or so). I think we could also use the same code on x86, but then we NEED to compile macos versions with clang.

It's probably better to keep the architecture conditional, if it doesn't provide any actual values for x86 it would only be extra binary bloat.

[joske]

@aristocratos The code was copied from htop, but this code is not yet merged into htop, it's from an open PR. Also, it's not a verbatim copy anymore as I changed the logic to return the highest freq and not the average. Not sure if that is enough. I don't know where they got this code from, I could only find a decompiled version of powermetrics which is similar, but doesn't do all the things this code does. It's completely undocumented.

For x86, I meant that right now, it's not useful, and this new code will likely work (not yet tested) on x86 too. I thought on x86, the old sysctl was returning useful values, but it doesn't it's a static value. I'll test if it works on x86 (it should, there's not really any arch dependent code). I only added the arch dependency to fix the CI build (plus I thought on x86 it actually worked). It seems we should just remove the old sysctl, as it just doesn't give any benefit.

[aristocratos]

@joske

it's from an open PR. Also, it's not a verbatim copy anymore as I changed the logic to return the highest freq and not the average. Not sure if that is enough.

Probably, but I'll leave it up to you if you think it's enough. (The worst that could happen is that we're asked to remove the code...)

For x86, I meant that right now, it's not useful, and this new code will likely work (not yet tested) on x86 too.

Ah, got it.

[joske]

Update: it does not work on x86, and htop also limits it to aarch64 😞.

[joske]

Good and Bad news. First the bad, the author of the htop code found out this code doesn't work correctly on Mx pro/max/ultra and is working on a fix.

I'll keep this as a draft until fully working.

The good news: he's willing to dual license it so we can use it 🎉