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

feat(cpu, mem, sensors, disk, process)(darwin): cgo-free implementations #1702

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

uubulb
Copy link
Contributor

@uubulb uubulb commented Sep 4, 2024

This PR provides cgo-free implementations for the following packages: cpu, mem, sensors, disk, process
This has been achieved by porting C structs to Go structs and calling C functions using purego.

Additionally, I have modified the behavior of sensors.TemperaturesWithContext on darwin arm64 to display the complete sensor name and return only a single value per sensor.
You can view the differences here: https://gist.github.com/uubulb/d75e5c7f490c14c398987f9a9a898a03

Tested on Mac15,6 (arm64, 14.6.1) and MacBookPro7,1 (amd64, 10.13.6).

Changes in interval/common/common_darwin.go are preparatory for further porting, but the current approach is somewhat complex. If anyone has better ideas, please share them!


Update: Added ports for disk and process, and removed go-m1cpu as a dependency since it requires cgo.

Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes #1700.

Great contribution!! It works like a charm on my Scaleway Mac m1 server.

However, the mac environment is widely used by many people. If possible, it would be greatly appreciated if a few more users could test it on their systems and provide feedback.

I wrote my results in detail.

## cpu

Current

=== RUN   TestTimes
    cpu_test.go:65: {"cpu":"cpu-total","user":153.7,"system":142.5,"idle":3712.8,"nice":0.0,"iowait":0.0,"irq":0.0,"softirq":0.0,"steal":0.0,"guest":0.0,"guestNice":0.0}
=== RUN   TestCounts
    cpu_test.go:90: logical cores: 8
    cpu_test.go:99: physical cores: 8

This PR

    cpu_test.go:65: {"cpu":"cpu-total","user":152.0,"system":140.6,"idle":3489.3,"nice":0.0,"iowait":0.0,"irq":0.0,"softirq":0.0,"steal":0.0,"guest":0.0,"guestNice":0.0}
=== RUN   TestCounts
    cpu_test.go:90: logical cores: 8
    cpu_test.go:99: physical cores: 8

mem

Current

{
  "total": 8589934592,
  "available": 5473402880,
  "used": 3116531712,
  "usedPercent": 36.28120422363281,
  "free": 2169143296,
  "active": 1928331264,
  "inactive": 3304259584,
  "wired": 638500864,
(snip)
}

This PR

{
  "total": 8589934592,
  "available": 5418008576,
  "used": 3171926016,
  "usedPercent": 36.92607879638672,
  "free": 2107785216,
  "active": 1926856704,
  "inactive": 3310223360,
  "wired": 696434688,
(snip)

sensors

Current

[{"sensorKey":"PMU","temperature":51.850006103515625,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU2","temperature":51.850006103515625,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMGR","temperature":26.28125,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU2","temperature":34.8245849609375,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"pACC","temperature":24.140625,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU","temperature":34.93186950683594,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMGR","temperature":26.421875,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU","temperature":21.647308349609375,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU","temperature":35.25372314453125,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"GPU","temperature":30,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"pACC","temperature":23.359375,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"SOC","temperature":27.09375,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU","temperature":29.470428466796875,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU","temperature":34.8245849609375,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU","temperature":28.689361572265625,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"eACC","temperature":21.625,"sensorHigh":0,"sensorCritical":0} {"sensorKey":"PMU","temperature":34.39544677734375,"sensorHigh":0,"sensorCritical":0} 
(snip)

This PR

[{"sensorKey":"PMU2 tdev4","temperature":27.834320068359375,"sensorHigh":0,"sensorCritical":0} 
{"sensorKey":"PMU2 tdie4","temperature":34.8245849609375,"sensorHigh":0,"sensorCritical":0} 
{"sensorKey":"pACC MTR Temp Sensor7","temperature":22.5625,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"pACC MTR Temp Sensor5","temperature":26.078125,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"SOC MTR Temp Sensor0","temperature":22.59375,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"PMU tdie5","temperature":33.1080322265625,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"PMU tdev8","temperature":29.115386962890625,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"PMU tdie1","temperature":34.610015869140625,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"ISP MTR Temp Sensor5","temperature":30,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"PMGR SOC Die Temp Sensor2","temperature":26.21875,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"pACC MTR Temp Sensor9","temperature":22.75,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"PMU tdev4","temperature":-21.621734619140625,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"PMGR SOC Die Temp Sensor1","temperature":26.421875,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"PMU2 tdie8","temperature":34.8245849609375,"sensorHigh":0,"sensorCritical":0}
{"sensorKey":"PMU tdev6","temperature":28.189361572265625,"sensorHigh":0,"sensorCritical":0}
(snip)

Additionally, I have modified the behavior of sensors.TemperaturesWithContext on darwin arm64 to display the complete sensor name and return only a single value per sensor.

I understand that the sensorkey implementation differs from the current one. However, the implementation in this PR seems preferable as it provides more detailed information. I plan to note this as a "Breaking Change" in the release notes.

Changes in interval/common/common_darwin.go are preparatory for further porting, but the current approach is somewhat complex. If anyone has better ideas, please share them!

It might indeed be a bit complex, but I believe the abstracted design using the Library struct makes sense. We might refactor it in the future, but for now, I don't see it as a major issue.

gopsutil has been developed with a focus on running across various architectures of Go and making cross-compilation easy. Therefore, I have always been concerned about cgo dependencies on Darwin. This PR addresses a long-standing concern. It's truly amazing. I deeply appreciate your contribution.

@uubulb uubulb changed the title feat(cpu, mem, sensors)(darwin): cgo-free implementations feat(cpu, mem, sensors, disk, process)(darwin): cgo-free implementations Sep 19, 2024
Copy link
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No one has done additional verification, but I have determined that it is probably fine and will proceed with the merge and will be included in the next release.

Thank you very much for your contribution, which represents a significant step forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants