expvar: revise API.

Nuke RemoveAll from the public API.
Replace Iter functions with Do functions.

Fixes #2852.

R=rsc, r
CC=golang-dev
https://golang.org/cl/5622055

diff --git a/doc/go1.html b/doc/go1.html
index 07adb677b0c8e3e584c3d13ccd3db343c4b83e2c..3f72831b057ecb340f395ba76f5d71c712a8c2d4 100644 (file)
--- a/doc/go1.html
+++ b/doc/go1.html
@@ -913,6 +913,23 @@ to be implemented in the future.
 No changes will be needed.
 </p>
 
+<h3 id="expvar">The expvar package</h3>
+
+<p>
+In Go 1, the <code>RemoveAll</code> function has been removed.
+The <code>Iter</code> function and Iter method on <code>*Map</code> have
+been replaced by
+<a href="/pkg/expvar/#Do"><code>Do</code></a>
+and
+<a href="/pkg/expvar/#Map.Do"><code>(*Map).Do</code></a>.
+</p>
+
+<p>
+<em>Updating</em>:
+Most code using <code>expvar</code> will not need changing. The rare code that used
+<code>Iter</code> can be updated to pass a closure to Do to achieve the same effect.
+</p>
+
 <h3 id="flag">The flag package</h3>
 
 <p>

diff --git a/doc/go1.tmpl b/doc/go1.tmpl
index 3287e137f28fc56652e862d81e864b028318dc9a..c4f486bac301666511fde4f196a080bcd7df3cec 100644 (file)
--- a/doc/go1.tmpl
+++ b/doc/go1.tmpl
@@ -817,6 +817,23 @@ to be implemented in the future.
 No changes will be needed.
 </p>
 
+<h3 id="expvar">The expvar package</h3>
+
+<p>
+In Go 1, the <code>RemoveAll</code> function has been removed.
+The <code>Iter</code> function and Iter method on <code>*Map</code> have
+been replaced by
+<a href="/pkg/expvar/#Do"><code>Do</code></a>
+and
+<a href="/pkg/expvar/#Map.Do"><code>(*Map).Do</code></a>.
+</p>
+
+<p>
+<em>Updating</em>:
+Most code using <code>expvar</code> will not need changing. The rare code that used
+<code>Iter</code> can be updated to pass a closure to Do to achieve the same effect.
+</p>
+
 <h3 id="flag">The flag package</h3>
 
 <p>

diff --git a/src/pkg/expvar/expvar.go b/src/pkg/expvar/expvar.go
index 5ced285804c1c16496ff05f3d0fc495fd7c9eaff..ee32eff9ea3bee7f5b51c640b3b5c4dc914c20a7 100644 (file)
--- a/src/pkg/expvar/expvar.go
+++ b/src/pkg/expvar/expvar.go
@@ -83,7 +83,7 @@ func (v *Float) Set(value float64) {
 // Map is a string-to-Var map variable that satisfies the Var interface.
 type Map struct {
        m  map[string]Var
-       mu sync.Mutex
+       mu sync.RWMutex
 }
 
 // KeyValue represents a single entry in a Map.
@@ -93,8 +93,8 @@ type KeyValue struct {
 }
 
 func (v *Map) String() string {
-       v.mu.Lock()
-       defer v.mu.Unlock()
+       v.mu.RLock()
+       defer v.mu.RUnlock()
        b := new(bytes.Buffer)
        fmt.Fprintf(b, "{")
        first := true
@@ -115,8 +115,8 @@ func (v *Map) Init() *Map {
 }
 
 func (v *Map) Get(key string) Var {
-       v.mu.Lock()
-       defer v.mu.Unlock()
+       v.mu.RLock()
+       defer v.mu.RUnlock()
        return v.m[key]
 }
 
@@ -127,12 +127,17 @@ func (v *Map) Set(key string, av Var) {
 }
 
 func (v *Map) Add(key string, delta int64) {
-       v.mu.Lock()
-       defer v.mu.Unlock()
+       v.mu.RLock()
        av, ok := v.m[key]
+       v.mu.RUnlock()
        if !ok {
-               av = new(Int)
-               v.m[key] = av
+               // check again under the write lock
+               v.mu.Lock()
+               if _, ok = v.m[key]; !ok {
+                       av = new(Int)
+                       v.m[key] = av
+               }
+               v.mu.Unlock()
        }
 
        // Add to Int; ignore otherwise.
@@ -143,12 +148,17 @@ func (v *Map) Add(key string, delta int64) {
 
 // AddFloat adds delta to the *Float value stored under the given map key.
 func (v *Map) AddFloat(key string, delta float64) {
-       v.mu.Lock()
-       defer v.mu.Unlock()
+       v.mu.RLock()
        av, ok := v.m[key]
+       v.mu.RUnlock()
        if !ok {
-               av = new(Float)
-               v.m[key] = av
+               // check again under the write lock
+               v.mu.Lock()
+               if _, ok = v.m[key]; !ok {
+                       av = new(Float)
+                       v.m[key] = av
+               }
+               v.mu.Unlock()
        }
 
        // Add to Float; ignore otherwise.
@@ -157,18 +167,15 @@ func (v *Map) AddFloat(key string, delta float64) {
        }
 }
 
-// TODO(rsc): Make sure map access in separate thread is safe.
-func (v *Map) iterate(c chan<- KeyValue) {
+// Do calls f for each entry in the map.
+// The map is locked during the iteration,
+// but existing entries may be concurrently updated.
+func (v *Map) Do(f func(KeyValue)) {
+       v.mu.RLock()
+       defer v.mu.RUnlock()
        for k, v := range v.m {
-               c <- KeyValue{k, v}
+               f(KeyValue{k, v})
        }
-       close(c)
-}
-
-func (v *Map) Iter() <-chan KeyValue {
-       c := make(chan KeyValue)
-       go v.iterate(c)
-       return c
 }
 
 // String is a string variable, and satisfies the Var interface.
@@ -190,8 +197,10 @@ func (f Func) String() string {
 }
 
 // All published variables.
-var vars map[string]Var = make(map[string]Var)
-var mutex sync.Mutex
+var (
+       mutex sync.RWMutex
+       vars  map[string]Var = make(map[string]Var)
+)
 
 // Publish declares a named exported variable. This should be called from a
 // package's init function when it creates its Vars. If the name is already
@@ -207,17 +216,11 @@ func Publish(name string, v Var) {
 
 // Get retrieves a named exported variable.
 func Get(name string) Var {
+       mutex.RLock()
+       defer mutex.RUnlock()
        return vars[name]
 }
 
-// RemoveAll removes all exported variables.
-// This is for tests; don't call this on a real server.
-func RemoveAll() {
-       mutex.Lock()
-       defer mutex.Unlock()
-       vars = make(map[string]Var)
-}
-
 // Convenience functions for creating new exported variables.
 
 func NewInt(name string) *Int {
@@ -244,31 +247,28 @@ func NewString(name string) *String {
        return v
 }
 
-// TODO(rsc): Make sure map access in separate thread is safe.
-func iterate(c chan<- KeyValue) {
+// Do calls f for each exported variable.
+// The global variable map is locked during the iteration,
+// but existing entries may be concurrently updated.
+func Do(f func(KeyValue)) {
+       mutex.RLock()
+       defer mutex.RUnlock()
        for k, v := range vars {
-               c <- KeyValue{k, v}
+               f(KeyValue{k, v})
        }
-       close(c)
-}
-
-func Iter() <-chan KeyValue {
-       c := make(chan KeyValue)
-       go iterate(c)
-       return c
 }
 
 func expvarHandler(w http.ResponseWriter, r *http.Request) {
        w.Header().Set("Content-Type", "application/json; charset=utf-8")
        fmt.Fprintf(w, "{\n")
        first := true
-       for name, value := range vars {
+       Do(func(kv KeyValue) {
                if !first {
                        fmt.Fprintf(w, ",\n")
                }
                first = false
-               fmt.Fprintf(w, "%q: %s", name, value)
-       }
+               fmt.Fprintf(w, "%q: %s", kv.Key, kv.Value)
+       })
        fmt.Fprintf(w, "\n}\n")
 }
 
@@ -281,7 +281,7 @@ func memstats() interface{} {
 }
 
 func init() {
-       http.Handle("/debug/vars", http.HandlerFunc(expvarHandler))
+       http.HandleFunc("/debug/vars", expvarHandler)
        Publish("cmdline", Func(cmdline))
        Publish("memstats", Func(memstats))
 }

diff --git a/src/pkg/expvar/expvar_test.go b/src/pkg/expvar/expvar_test.go
index fc607274b280062572e74895ca57108481ee644a..bbd9dd8d6e94308d085cff3e5ea3c5b0cb7c19c4 100644 (file)
--- a/src/pkg/expvar/expvar_test.go
+++ b/src/pkg/expvar/expvar_test.go
@@ -9,6 +9,14 @@ import (
        "testing"
 )
 
+// RemoveAll removes all exported variables.
+// This is for tests only.
+func RemoveAll() {
+       mutex.Lock()
+       defer mutex.Unlock()
+       vars = make(map[string]Var)
+}
+
 func TestInt(t *testing.T) {
        reqs := NewInt("requests")
        if reqs.i != 0 {