cmd/juju/machine: Enable GOMAXPROCS(NumCPU)

utils/gomaxprocs.go

 // Copyright 2014 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package utils
 
 import (
         "os"
         "runtime"
-        "sync"
 )
 
-var mu = sync.Mutex{}

[rog, 2014/03/17 10:02:16]

This does not fill me with joy.
If our tests really do not play nicely with GOMAXPROCS>1,
(they *really* should, but I understand that they
might not) there's a much easier way to override it - just set
the environment variable.

However, for the jujud tests specifically, the tests
pass fine for me when jujud sets GOMAXPROCS.

I suggest that we add just implement the following
function in cmd/jujud
(I don't think it's generally useful enough to deserve a place in utils,
but it can go here if you think it's worth it):

func UseMultipleCPUs() {
   if os.Getenv("GOMAXPROCS") == "" {
       runtime.GOMAXPROCS(runtime.NumCPU())
   }
}


and call it from the same place it's called currently.
To test it, you could declare it as a variable and
replace it to ensure it's actually called.

If we really find that it makes the tests flaky,
the relevant tests can patch $GOMAXPROCS.

[jameinel, 2014/03/19 09:45:27]

Done.

 var gomaxprocs = runtime.GOMAXPROCS
 var numcpu = runtime.NumCPU

[rog, 2014/03/20 08:43:06]

Trivial point that I forgot to mention before:
we'd conventionally spell this numCPU.

I'd be tempted to use goMaxProcs too, but I see
the rationale for all lower case there too.

-var enabledMultiCPUs = false
 
-// OverrideGOMAXPROCSFuncs allows you to override calling runtime.GOMAXPROCS
-// and runtime.NumCPU. This is exposed so that tests can poke at this without
-// actually changing the runtime behavior.
-func OverrideGOMAXPROCSFuncs(newGOMAXPROCS func(int) int, newNumCPU func() int) (cleanup func()) {
-        mu.Lock()
-        defer mu.Unlock()
-        origGOMAXPROCS := gomaxprocs
-        logger.Debugf("setting GOMAXPROCS to %#v", newGOMAXPROCS)
-        gomaxprocs = newGOMAXPROCS
-        origNumCPU := numcpu
-        numcpu = newNumCPU
-        return func() {
-                mu.Lock()
-                defer mu.Unlock()
-                gomaxprocs = origGOMAXPROCS
-                numcpu = origNumCPU
-                enabledMultiCPUs = false
-        }
-}
-
-// EnableMultipleCPUs is called when we want to allow the system to have
-// GOMAXPROCS>1. By default we only want to enable GOMAXPROCS>1 when running in
-// the jujud machine agents that are serving the API. We don't want to set it
-// in the test suite, etc.
-// So first you call EnableMultipleCPUs() when we determine we are running from
-// main(), and then later on you call UseMultipleCPUs() when we determine we
-// are running a machine agent that is serving the API.
-func EnableMultipleCPUs() {
-        mu.Lock()
-        defer mu.Unlock()
-        // We check to see if GOMAXPROCS is set in the environment. If it is,
-        // then we will just use the environment variable, and not override it
-        // ourselves
-        if os.Getenv("GOMAXPROCS") == "" {
-                enabledMultiCPUs = true
-        }
-}
-
-// UseMultipleCPUs is called when we have decided we want to set GOMAXPROCS to
-// the current number of CPU cores. This will not override the environment
-// variable if it is set.
+// UseMultipleCPUs sets GOMAXPROCS to the number of CPU cores unless it has
+// already been overridden by the GOMAXPROCS environment variable.
 func UseMultipleCPUs() {
-        mu.Lock()
-        defer mu.Unlock()
-        if !enabledMultiCPUs {
-                logger.Debugf("multiple CPUs not enabled, calling GOMAXPROCS(0)")
-                gomaxprocs(0)
+        if envGOMAXPROCS := os.Getenv("GOMAXPROCS"); envGOMAXPROCS != "" {
+                n := gomaxprocs(0)
+                logger.Debugf("GOMAXPROCS already set in environment to %q, %d internally",
+                        envGOMAXPROCS, n)
                 return
         }
         n := numcpu()
         logger.Debugf("setting GOMAXPROCS to %d", n)
         gomaxprocs(n)
 }