Skip to content

Commit

Permalink
Minimum GOMAXPROCS should be 1 (#10)
Browse files Browse the repository at this point in the history
We previously set the minimum GOMAXPROCS value to 2 as that was the
minimum allocation we expected internally. However, if the allocation is
actually smaller, we should use lower GOMAXPROCS.

This will also allow us to reduce the allocation size and use a more
appropriate GOMAXPROCS which will reduce the amount of CFS throttling.
  • Loading branch information
prashantv committed Nov 10, 2017
1 parent 2b02b99 commit 4225287
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 9 deletions.
3 changes: 0 additions & 3 deletions internal/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,3 @@ const (
// CPUQuotaMinUsed is return when CPU quota is larger than the min value
CPUQuotaMinUsed
)

// MinGOMAXPROCS defines the minimum value for GOMAXPROCS
const MinGOMAXPROCS = 2
22 changes: 18 additions & 4 deletions maxprocs/maxprocs.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ func currentMaxProcs() int {
}

type config struct {
printf func(string, ...interface{})
procs func(int) (int, iruntime.CPUQuotaStatus, error)
printf func(string, ...interface{})
procs func(int) (int, iruntime.CPUQuotaStatus, error)
minGOMAXPROCS int
}

func (c *config) log(fmt string, args ...interface{}) {
Expand All @@ -60,6 +61,16 @@ func Logger(printf func(string, ...interface{})) Option {
})
}

// Min sets the minimum GOMAXPROCS value that will be used.
// Any value below 1 is ignored.
func Min(n int) Option {
return optionFunc(func(cfg *config) {
if n >= 1 {
cfg.minGOMAXPROCS = n
}
})
}

type optionFunc func(*config)

func (of optionFunc) apply(cfg *config) { of(cfg) }
Expand All @@ -70,7 +81,10 @@ func (of optionFunc) apply(cfg *config) { of(cfg) }
// Set is a no-op on non-Linux systems and in Linux environments without a
// configured CPU quota.
func Set(opts ...Option) (func(), error) {
cfg := &config{procs: iruntime.CPUQuotaToGOMAXPROCS}
cfg := &config{
procs: iruntime.CPUQuotaToGOMAXPROCS,
minGOMAXPROCS: 1,
}
for _, o := range opts {
o.apply(cfg)
}
Expand All @@ -87,7 +101,7 @@ func Set(opts ...Option) (func(), error) {
return undoNoop, nil
}

maxProcs, status, err := cfg.procs(iruntime.MinGOMAXPROCS)
maxProcs, status, err := cfg.procs(cfg.minGOMAXPROCS)
if err != nil {
return undoNoop, err
}
Expand Down
18 changes: 16 additions & 2 deletions maxprocs/maxprocs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,29 @@ func TestSet(t *testing.T) {
quotaOpt := stubProcs(func(min int) (int, iruntime.CPUQuotaStatus, error) {
return min, iruntime.CPUQuotaMinUsed, nil
})
undo, err := Set(logOpt, quotaOpt)
undo, err := Set(logOpt, quotaOpt, Min(5))
defer undo()
require.NoError(t, err, "Set failed")
assert.Equal(t, 5, currentMaxProcs(), "should use min allowed GOMAXPROCS")
assert.Contains(t, buf.String(), "using minimum allowed", "unexpected log output")
})

t.Run("Min unused", func(t *testing.T) {
buf, logOpt := testLogger()
quotaOpt := stubProcs(func(min int) (int, iruntime.CPUQuotaStatus, error) {
return min, iruntime.CPUQuotaMinUsed, nil
})
// Min(-1) should be ignored.
undo, err := Set(logOpt, quotaOpt, Min(5), Min(-1))
defer undo()
require.NoError(t, err, "Set failed")
assert.Equal(t, iruntime.MinGOMAXPROCS, currentMaxProcs(), "should use min allowed GOMAXPROCS")
assert.Equal(t, 5, currentMaxProcs(), "should use min allowed GOMAXPROCS")
assert.Contains(t, buf.String(), "using minimum allowed", "unexpected log output")
})

t.Run("QuotaUsed", func(t *testing.T) {
opt := stubProcs(func(min int) (int, iruntime.CPUQuotaStatus, error) {
assert.Equal(t, 1, min, "Default minimum value should be 1")
return 42, iruntime.CPUQuotaUsed, nil
})
undo, err := Set(opt)
Expand Down

0 comments on commit 4225287

Please sign in to comment.