Apple Silicon GPU support #1470
Conversation
|
So this adds a powemetrics api, and then removes it? |
deckstose
left a comment
There was a problem hiding this comment.
Most of the code is not properly formatted.
There is a huge amount of AI assisted code in there, this is what I was getting from the issue comments (?).
Most of the code needs COMMENTS. Especially the function pointer magic.
|
I had kept the power metrics to read the CPU data. But I think it's outside the scope to just add the GPU. I removed it for now. With a little work, you can get the data via IOKit. I took Emanuele Zattin's code, which was allegedly written by him using AI, and reworked some things that I thought were relevant. I can redo it by scheduling with AI. |
2e6951e to
215cbe0
Compare
|
I added some comments, but I think it might not be enough, and I haven't formatted it properly yet. |
|
I don't care about the boolean operators Also IMO comments should not include the question mark (although it's done in other parts of the code). It's fine either way, but redundant in a way. And I'm not sure if editors with their commenting feature support it. |
whoops that how I was commenting code here because almost all the comments I already saw in the code had it. I will keep that in mind in the future |
|
No worries, I have merged code with this style |
|
Please mark this ready for review when this is ready |
0331902 to
0321cd7
Compare
|
I made the changes, but I'm not sure if it's any good. If you could take a look |
|
I will take a look but it's not going to be anytime soon |
There was a problem hiding this comment.
Pull request overview
This PR adds GPU monitoring support for Apple Silicon Macs by leveraging IOKit framework to collect GPU metrics without requiring root privileges. This addresses issue #701 by providing a sudoless alternative to powermetrics for GPU monitoring on Apple Silicon.
Changes:
- Added new IOKit utility framework for CoreFoundation object handling and IOService iteration
- Implemented Apple Silicon GPU monitoring via IOAccelerator and IOReport frameworks
- Refactored existing code to use new safe helper functions for CF object conversions
- Integrated GPU support into build systems (Makefile and CMake)
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| src/osx/smc.hpp | Removed unused masterPort variable |
| src/osx/sensors.cpp | Refactored to use new safe CF conversion helpers and moved HID declarations |
| src/osx/iokit.hpp | New header defining IOKit utility functions and IOReport framework bindings |
| src/osx/iokit.cpp | Implementation of safe CF converters and IOService iterators |
| src/osx/gpu.hpp | New header defining GPU monitoring classes (GPU, GPUActivities, IOGPU) |
| src/osx/gpu.cpp | Implementation of GPU metrics collection via IOAccelerator and IOReport |
| src/osx/btop_collect.cpp | Integrated GPU collection into main collection loop |
| src/btop_shared.hpp | Added Apple-specific shutdown namespace for GPU support |
| src/btop_config.cpp | Added "apple" to default shown_gpus config on macOS |
| src/btop.cpp | Added IOAccelerator shutdown call for macOS |
| Makefile | Enabled GPU_SUPPORT for macOS ARM64 |
| CMakeLists.txt | Added iokit.cpp and gpu.cpp sources, enabled GPU_SUPPORT definition |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Seems to work as intended on a Mac Mini M1 running Tahoe. But would be great to have some testing done on other Mac hardware. What of the code is AI generated? Because that code would need to be checked so it hasn't been "lifted" from some other project (and might have missing attribution or incompatible licensing). |
My hardware is an M4 Pro, and that's where I tested it. Regarding the code being marked by AI, and because of Emanuele Zattin's contribution, parts of what the code estimates to be CPU usage were inspired by AI. I say inspired because I didn't request a code per se, but rather the procedure. Our reference code is https://github.com/dehydratedpotato/socpowerbud |
There was a problem hiding this comment.
I've pointed out a couple of patterns, they should apply everywhere.
I can't say much about the actual implementation, but it looks good over all.
I wonder if it is necessary to have a struct indicating no GPU support, the undefined GPU_SUPPORT macro should be enough, right?
Lines 192 to 196 in 0c11d73
EDIT: Resolved in #1495.
|
|
||
|
|
||
| namespace Gpu { | ||
| #ifdef GPU_SUPPORT |
There was a problem hiding this comment.
Namespace should be inside ifdef
| namespace rng = std::ranges; | ||
| using namespace Tools; | ||
|
|
||
|
|
| #include "sensors.hpp" | ||
| #endif | ||
| #include "smc.hpp" | ||
|
|
| Mem::old_uptime = system_uptime(); | ||
| Mem::collect(); | ||
|
|
||
| #ifdef GPU_SUPPORT |
There was a problem hiding this comment.
Please put macros at the start of the line.
| }; | ||
|
|
||
| if (auto it = map.find(key); it != map.end()) { | ||
| usage.*(it->second) = value; |
There was a problem hiding this comment.
This needs to be rewritten in a more readable syntax.
| proc = std::stoul(s.substr(pidPos + 4, commaPos - (pidPos + 4))); | ||
| name = s.substr(commaPos + 2); | ||
| } catch (...) { | ||
| return; |
| if(APPLE AND BTOP_GPU) | ||
| target_compile_definitions(libbtop PUBLIC GPU_SUPPORT) | ||
| endif() |
There was a problem hiding this comment.
Combine this branch with the Linux branch with GPU_SUPPORT defined
This comment was marked as outdated.
This comment was marked as outdated.
a19adf5 to
4348f11
Compare
|
CI is failing |
Fixed |
29e72ce to
c84ca8b
Compare
c84ca8b to
de3450f
Compare



With help from the folks at Issues #701 , I organized it using only IOKit, which provides the data without root.
It has only been tested on my M4 Pro platform, so anyone willing to test it would be very welcome.
Apparently, we can use the IOReport framework to obtain energy data for the CPU, such as usage in watts and clock speed.
We can also use the same framework to add the ANE (Apple Neural Engine).