Change exvar to use a goroutine channel worker instead of a mutex for synchronisation.

Also it should be more testable, as there's less global state.

R=r
APPROVED=r
DELTA=113  (38 added, 12 deleted, 63 changed)
OCL=27653
CL=27694

src/lib/Makefile

@@ -99,7 +99,7 @@ test: test.files
 bignum.6: fmt.dirinstall
 bufio.6: io.dirinstall os.dirinstall
 exec.6: os.dirinstall strings.install
-exvar.6: fmt.dirinstall sync.dirinstall
+exvar.6: fmt.dirinstall
 flag.6: fmt.dirinstall os.dirinstall strconv.dirinstall
 log.6: fmt.dirinstall io.dirinstall os.dirinstall time.dirinstall
 path.6: io.dirinstall

src/lib/exvar.go

@@ -8,7 +8,6 @@ package exvar
 
 import (
 	"fmt";
-	"sync";
 )
 
 // If mismatched names are used (e.g. calling IncrementInt on a mapVar), the
@@ -49,112 +48,139 @@ func (m mapVar) String() string {
 // - string-valued vars
 // - dynamic lookup vars (via chan?)
 
-// Global state.
-var (
-	mutex sync.Mutex;
-	vars = make(map[string] exVar);
+type exVars struct {
+	vars map[string] exVar;
 	// TODO(dsymonds): docstrings
-)
+}
+
+// Singleton worker goroutine.
+// Functions needing access to the global state have to pass a closure to the
+// worker channel, which is read by a single workerFunc running in a goroutine.
+// Nil values are silently ignored, so you can send nil to the worker channel
+// after the closure if you want to block until your work is done. This risks
+// blocking you, though. The workSync function wraps this as a convenience.
+
+type workFunction func(*exVars);
+
+// The main worker function that runs in a goroutine.
+// It never ends in normal operation.
+func startWorkerFunc() <-chan workFunction {
+	ch := make(chan workFunction);
+
+	state := &exVars{ make(map[string] exVar) };
+
+	go func() {
+		for f := range ch {
+			if f != nil {
+				f(state)
+			}
+		}
+	}();
+	return ch
+}
+
+var worker = startWorkerFunc();
+
+// workSync will enqueue the given workFunction and wait for it to finish.
+func workSync(f workFunction) {
+	worker <- f;
+	worker <- nil  // will only be sent after f() completes.
+}
 
 // getOrInitIntVar either gets or initializes an intVar called name.
-// Callers should already be holding the mutex.
-func getOrInitIntVar(name string) *intVar {
-	if v, ok := vars[name]; ok {
+func (state *exVars) getOrInitIntVar(name string) *intVar {
+	if v, ok := state.vars[name]; ok {
 		// Existing var
 		if iv, ok := v.(*intVar); ok {
 			return iv
 		}
 		// Type mismatch.
-		return getOrInitIntVar(mismatchedInt)
+		return state.getOrInitIntVar(mismatchedInt)
 	}
 	// New var
 	iv := new(intVar);
-	vars[name] = iv;
+	state.vars[name] = iv;
 	return iv
 }
 
 // getOrInitMapVar either gets or initializes a mapVar called name.
-// Callers should already be holding the mutex.
-func getOrInitMapVar(name string) *mapVar {
-	if v, ok := vars[name]; ok {
+func (state *exVars) getOrInitMapVar(name string) *mapVar {
+	if v, ok := state.vars[name]; ok {
 		// Existing var
 		if mv, ok := v.(*mapVar); ok {
 			return mv
 		}
 		// Type mismatch.
-		return getOrInitMapVar(mismatchedMap)
+		return state.getOrInitMapVar(mismatchedMap)
 	}
 	// New var
 	var m mapVar = make(map[string] int);
-	vars[name] = &m;
+	state.vars[name] = &m;
 	return &m
 }
 
 // IncrementInt adds inc to the integer-valued var called name.
 func IncrementInt(name string, inc int) {
-	mutex.Lock();
-	defer mutex.Unlock();
-
-	*getOrInitIntVar(name) += inc
+	workSync(func(state *exVars) {
+		*state.getOrInitIntVar(name) += inc
+	})
 }
 
-// IncrementMap adds inc to the keyed value in the map-valued var called name.
-func IncrementMap(name string, key string, inc int) {
-	mutex.Lock();
-	defer mutex.Unlock();
-
-	mv := getOrInitMapVar(name);
-	// TODO(dsymonds): Change this to just mv[key] when bug143 is fixed.
-	if v, ok := (*mv)[key]; ok {
-		mv[key] += inc
-	} else {
-		mv[key] = inc
-	}
+// IncrementMapInt adds inc to the keyed value in the map-valued var called name.
+func IncrementMapInt(name string, key string, inc int) {
+	workSync(func(state *exVars) {
+		mv := state.getOrInitMapVar(name);
+		// TODO(dsymonds): Change this to just mv[key] when bug143 is fixed.
+		if v, ok := (*mv)[key]; ok {
+			mv[key] += inc
+		} else {
+			mv[key] = inc
+		}
+	})
 }
 
 // SetInt sets the integer-valued var called name to value.
 func SetInt(name string, value int) {
-	mutex.Lock();
-	defer mutex.Unlock();
-
-	*getOrInitIntVar(name) = value
+	workSync(func(state *exVars) {
+		*state.getOrInitIntVar(name) = value
+	})
 }
 
-// SetMap sets the keyed value in the map-valued var called name.
-func SetMap(name string, key string, value int) {
-	mutex.Lock();
-	defer mutex.Unlock();
-
-	getOrInitMapVar(name)[key] = value
+// SetMapInt sets the keyed value in the map-valued var called name.
+func SetMapInt(name string, key string, value int) {
+	workSync(func(state *exVars) {
+		state.getOrInitMapVar(name)[key] = value
+	})
 }
 
 // GetInt retrieves an integer-valued var called name.
 func GetInt(name string) int {
-	mutex.Lock();
-	defer mutex.Unlock();
-
-	return *getOrInitIntVar(name)
+	var i int;
+	workSync(func(state *exVars) {
+		i = *state.getOrInitIntVar(name)
+	});
+	return i
 }
 
-// GetMap retrieves the keyed value for a map-valued var called name.
-func GetMap(name string, key string) int {
-	mutex.Lock();
-	defer mutex.Unlock();
-
-	// TODO(dsymonds): Change this to just getOrInitMapVar(name)[key] when
-	// bug143 is fixed.
-	x, ok := (*getOrInitMapVar(name))[key];
-	return x
+// GetMapInt retrieves the keyed value for a map-valued var called name.
+func GetMapInt(name string, key string) int {
+	var i int;
+	var ok bool;
+	workSync(func(state *exVars) {
+		// TODO(dsymonds): Change this to just getOrInitMapVar(name)[key] when
+		// bug143 is fixed.
+		i, ok = (*state.getOrInitMapVar(name))[key];
+	});
+	return i
 }
 
 // String produces a string of all the vars in textual format.
 func String() string {
-	mutex.Lock();
-	defer mutex.Unlock();
-
 	s := "";
-	for name, value := range vars {
-		s += fmt.Sprintln(name, value)
-	}
+	workSync(func(state *exVars) {
+		for name, value := range state.vars {
+			s += fmt.Sprintln(name, value)
+		}
+	});
 	return s
 }

src/lib/exvar_test.go

@@ -34,33 +34,33 @@ func TestSimpleCounter(t *testing.T) {
 func TestMismatchedCounters(t *testing.T) {
 	// Make sure some vars exist.
 	GetInt("requests");
-	GetMap("colours", "red");
+	GetMapInt("colours", "red");
 
 	IncrementInt("colours", 1);
 	if x := GetInt("x-mismatched-int"); x != 1 {
 		t.Errorf("GetInt('x-mismatched-int') = %v, want 1", x)
 	}
 
-	IncrementMap("requests", "orange", 1);
-	if x := GetMap("x-mismatched-map", "orange"); x != 1 {
-		t.Errorf("GetMap('x-mismatched-int', 'orange') = %v, want 1", x)
+	IncrementMapInt("requests", "orange", 1);
+	if x := GetMapInt("x-mismatched-map", "orange"); x != 1 {
+		t.Errorf("GetMapInt('x-mismatched-int', 'orange') = %v, want 1", x)
 	}
 }
 
 func TestMapCounter(t *testing.T) {
 	// Unknown exvar should be zero.
-	if x := GetMap("colours", "red"); x != 0 {
-		t.Errorf("GetMap(non, existent) = %v, want 0", x)
+	if x := GetMapInt("colours", "red"); x != 0 {
+		t.Errorf("GetMapInt(non, existent) = %v, want 0", x)
 	}
 
-	IncrementMap("colours", "red", 1);
-	IncrementMap("colours", "red", 2);
-	IncrementMap("colours", "blue", 4);
-	if x := GetMap("colours", "red"); x != 3 {
-		t.Errorf("GetMap('colours', 'red') = %v, want 3", x)
+	IncrementMapInt("colours", "red", 1);
+	IncrementMapInt("colours", "red", 2);
+	IncrementMapInt("colours", "blue", 4);
+	if x := GetMapInt("colours", "red"); x != 3 {
+		t.Errorf("GetMapInt('colours', 'red') = %v, want 3", x)
 	}
-	if x := GetMap("colours", "blue"); x != 4 {
-		t.Errorf("GetMap('colours', 'blue') = %v, want 4", x)
+	if x := GetMapInt("colours", "blue"); x != 4 {
+		t.Errorf("GetMapInt('colours', 'blue') = %v, want 4", x)
 	}
 
 	// TODO(dsymonds): Test String()