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

diff --git a/src/lib/Makefile b/src/lib/Makefile
index 640f329ddf0d3ebd57928524479c7cf8f45a2fe8..2a3d76dcc5f4467d180246b20d9a7e7a9e718aea 100644 (file)
--- a/src/lib/Makefile
+++ b/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

diff --git a/src/lib/exvar.go b/src/lib/exvar.go
index 88f36b7bd81d20186d5d40c30c5b7fdecfde0dd4..38fd2c152b6a29673ae989a8fba22ab40d0d7727 100644 (file)
--- a/src/lib/exvar.go
+++ b/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
 }

diff --git a/src/lib/exvar_test.go b/src/lib/exvar_test.go
index 6b309f3a49e2229c2acca1bbe9548ec7d70927ff..8e9b123d0506db371231af198a176a08a3dd2c9a 100644 (file)
--- a/src/lib/exvar_test.go
+++ b/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()