Skip to content

metrics: disable CPU stats (gosigar) on Darwing when nocgo is passed #20829

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

metrics: disable CPU stats (gosigar) on Darwing when nocgo is passed #20829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0b6eabf
metrics: disable CPU stats (gosigar) on Darwing when nocgo is passed.
BogdanHabic Mar 27, 2020
5bd80cb
metrics: enable CPU stats (gosigar) on arm64.
BogdanHabic Mar 28, 2020
71f17d3
Changed the build constraints for darwin CPU stats when cgo is disabled.
BogdanHabic Apr 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions metrics/cpu_darwin_cgo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2020 The go-ethereum Authors
// This file is part of the go-ethereum library.
//
// The go-ethereum library is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// The go-ethereum library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.

// +build darwin,cgo

package metrics

import "github.com/elastic/gosigar"

// ReadCPUStats retrieves the current CPU stats.
func ReadCPUStats(stats *CPUStats) {
global := gosigar.Cpu{}
global.Get()

stats.GlobalTime = int64(global.User + global.Nice + global.Sys)
stats.GlobalWait = int64(global.Wait)
stats.LocalTime = getProcessCPUTime()
}
2 changes: 1 addition & 1 deletion metrics/cpu_disabled.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.

// +build ios
// +build ios darwin,!cgo

package metrics

Expand Down
2 changes: 1 addition & 1 deletion metrics/cpu_enabled.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU Lesser General Public License
// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.

// +build !ios
// +build !ios,!darwin
Copy link
Member

Choose a reason for hiding this comment

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

I think you should be able to cover all cases here and not need that extra file:

// +build !ios
// +build !darwin cgo

Could you try this version out?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be

Suggested change
// +build !ios,!darwin
// +build !ios,!darwin darwin,cgo

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the truth table I'm using

ios darwin Other OS cgo cpu enabled
0 1 0 0 0
0 1 0 1 1
1 0 0 0 0
1 0 0 1 0
0 0 1 0 1
0 0 1 1 1

Copy link
Author

Choose a reason for hiding this comment

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

So the Other OS will essentially be caught by !ios,!darwin, correct? If this is the case, the change you suggested makes sense.


package metrics

Expand Down