Skip to content

Commit fb0fe42

Browse files
author
Bryan C. Mills
committed
expvar: replace RWMutex usage with sync.Map and atomics
Int and Float already used atomics. When many goroutines on many CPUs concurrently update a StringSet or a Map with different keys per goroutine, this change results in dramatic steady-state speedups. This change does add some overhead for single-CPU and ephemeral maps. I believe that is mostly due to an increase in allocations per call (to pack the map keys and values into interface{} values that may escape into the heap). With better inlining and/or escape analysis, the single-CPU penalty may decline somewhat. There are still two RWMutexes in the package: one for the keys in the global "vars" map, and one for the keys in individual Map variables. Those RWMutexes could also be eliminated, but avoiding excessive allocations when adding new keys would require care. The remaining RWMutexes are only acquired in Do functions, which I believe are not typically on the fast path. updates #17973 updates #18177 name old time/op new time/op delta StringSet 65.9ns ± 8% 55.7ns ± 1% -15.46% (p=0.000 n=8+7) StringSet-6 416ns ±22% 127ns ±19% -69.37% (p=0.000 n=8+8) StringSet-48 309ns ± 8% 94ns ± 3% -69.43% (p=0.001 n=7+7) name old alloc/op new alloc/op delta StringSet 0.00B 16.00B ± 0% +Inf% (p=0.000 n=8+8) StringSet-6 0.00B 16.00B ± 0% +Inf% (p=0.000 n=8+8) StringSet-48 0.00B 16.00B ± 0% +Inf% (p=0.000 n=8+8) name old allocs/op new allocs/op delta StringSet 0.00 1.00 ± 0% +Inf% (p=0.000 n=8+8) StringSet-6 0.00 1.00 ± 0% +Inf% (p=0.000 n=8+8) StringSet-48 0.00 1.00 ± 0% +Inf% (p=0.000 n=8+8) https://perf.golang.org/search?q=upload:20170427.3 name old time/op new time/op delta IntAdd 5.64ns ± 3% 5.58ns ± 1% ~ (p=0.185 n=8+8) IntAdd-6 18.6ns ±32% 21.4ns ±21% ~ (p=0.078 n=8+8) IntAdd-48 19.6ns ±13% 20.6ns ±19% ~ (p=0.702 n=8+8) IntSet 5.50ns ± 1% 5.48ns ± 0% ~ (p=0.222 n=7+8) IntSet-6 18.5ns ±16% 20.4ns ±30% ~ (p=0.314 n=8+8) IntSet-48 19.7ns ±12% 20.4ns ±16% ~ (p=0.522 n=8+8) FloatAdd 14.5ns ± 1% 14.6ns ± 2% ~ (p=0.237 n=7+8) FloatAdd-6 69.9ns ±13% 68.4ns ± 7% ~ (p=0.557 n=7+7) FloatAdd-48 110ns ± 9% 109ns ± 6% ~ (p=0.667 n=8+8) FloatSet 7.62ns ± 3% 7.64ns ± 5% ~ (p=0.939 n=8+8) FloatSet-6 20.7ns ±22% 21.0ns ±23% ~ (p=0.959 n=8+8) FloatSet-48 20.4ns ±24% 20.8ns ±19% ~ (p=0.899 n=8+8) MapSet 88.1ns ±15% 200.9ns ± 7% +128.11% (p=0.000 n=8+8) MapSet-6 453ns ±12% 202ns ± 8% -55.43% (p=0.000 n=8+8) MapSet-48 432ns ±12% 240ns ±15% -44.49% (p=0.000 n=8+8) MapSetDifferent 349ns ± 1% 876ns ± 2% +151.08% (p=0.001 n=6+7) MapSetDifferent-6 1.74µs ±32% 0.25µs ±17% -85.71% (p=0.000 n=8+8) MapSetDifferent-48 1.77µs ±10% 0.14µs ± 2% -91.84% (p=0.000 n=8+8) MapSetString 88.1ns ± 7% 205.3ns ± 5% +132.98% (p=0.001 n=7+7) MapSetString-6 438ns ±30% 205ns ± 9% -53.15% (p=0.000 n=8+8) MapSetString-48 419ns ±14% 241ns ±15% -42.39% (p=0.000 n=8+8) MapAddSame 686ns ± 9% 1010ns ± 5% +47.41% (p=0.000 n=8+8) MapAddSame-6 238ns ±10% 300ns ±11% +26.22% (p=0.000 n=8+8) MapAddSame-48 366ns ± 4% 483ns ± 3% +32.06% (p=0.000 n=8+8) MapAddDifferent 1.96µs ± 4% 3.24µs ± 6% +65.58% (p=0.000 n=8+8) MapAddDifferent-6 553ns ± 3% 948ns ± 8% +71.43% (p=0.000 n=7+8) MapAddDifferent-48 548ns ± 4% 1242ns ±10% +126.81% (p=0.000 n=8+8) MapAddSameSteadyState 31.5ns ± 7% 41.7ns ± 6% +32.61% (p=0.000 n=8+8) MapAddSameSteadyState-6 239ns ± 7% 101ns ±30% -57.53% (p=0.000 n=7+8) MapAddSameSteadyState-48 152ns ± 4% 85ns ±13% -43.84% (p=0.000 n=8+7) MapAddDifferentSteadyState 151ns ± 5% 177ns ± 1% +17.32% (p=0.001 n=8+6) MapAddDifferentSteadyState-6 861ns ±15% 62ns ±23% -92.85% (p=0.000 n=8+8) MapAddDifferentSteadyState-48 617ns ± 2% 20ns ±14% -96.75% (p=0.000 n=8+8) RealworldExpvarUsage 4.33µs ± 4% 4.48µs ± 6% ~ (p=0.336 n=8+7) RealworldExpvarUsage-6 2.12µs ±20% 2.28µs ±10% ~ (p=0.228 n=8+6) RealworldExpvarUsage-48 1.23µs ±19% 1.36µs ±16% ~ (p=0.152 n=7+8) name old alloc/op new alloc/op delta IntAdd 0.00B 0.00B ~ (all equal) IntAdd-6 0.00B 0.00B ~ (all equal) IntAdd-48 0.00B 0.00B ~ (all equal) IntSet 0.00B 0.00B ~ (all equal) IntSet-6 0.00B 0.00B ~ (all equal) IntSet-48 0.00B 0.00B ~ (all equal) FloatAdd 0.00B 0.00B ~ (all equal) FloatAdd-6 0.00B 0.00B ~ (all equal) FloatAdd-48 0.00B 0.00B ~ (all equal) FloatSet 0.00B 0.00B ~ (all equal) FloatSet-6 0.00B 0.00B ~ (all equal) FloatSet-48 0.00B 0.00B ~ (all equal) MapSet 0.00B 48.00B ± 0% +Inf% (p=0.000 n=8+8) MapSet-6 0.00B 48.00B ± 0% +Inf% (p=0.000 n=8+8) MapSet-48 0.00B 48.00B ± 0% +Inf% (p=0.000 n=8+8) MapSetDifferent 0.00B 192.00B ± 0% +Inf% (p=0.000 n=8+8) MapSetDifferent-6 0.00B 192.00B ± 0% +Inf% (p=0.000 n=8+8) MapSetDifferent-48 0.00B 192.00B ± 0% +Inf% (p=0.000 n=8+8) MapSetString 0.00B 48.00B ± 0% +Inf% (p=0.000 n=8+8) MapSetString-6 0.00B 48.00B ± 0% +Inf% (p=0.000 n=8+8) MapSetString-48 0.00B 48.00B ± 0% +Inf% (p=0.000 n=8+8) MapAddSame 456B ± 0% 480B ± 0% +5.26% (p=0.000 n=8+8) MapAddSame-6 456B ± 0% 480B ± 0% +5.26% (p=0.000 n=8+8) MapAddSame-48 456B ± 0% 480B ± 0% +5.26% (p=0.000 n=8+8) MapAddDifferent 672B ± 0% 1088B ± 0% +61.90% (p=0.000 n=8+8) MapAddDifferent-6 672B ± 0% 1088B ± 0% +61.90% (p=0.000 n=8+8) MapAddDifferent-48 672B ± 0% 1088B ± 0% +61.90% (p=0.000 n=8+8) MapAddSameSteadyState 0.00B 0.00B ~ (all equal) MapAddSameSteadyState-6 0.00B 0.00B ~ (all equal) MapAddSameSteadyState-48 0.00B 0.00B ~ (all equal) MapAddDifferentSteadyState 0.00B 0.00B ~ (all equal) MapAddDifferentSteadyState-6 0.00B 0.00B ~ (all equal) MapAddDifferentSteadyState-48 0.00B 0.00B ~ (all equal) RealworldExpvarUsage 0.00B 0.00B ~ (all equal) RealworldExpvarUsage-6 0.00B 0.00B ~ (all equal) RealworldExpvarUsage-48 0.00B 0.00B ~ (all equal) name old allocs/op new allocs/op delta IntAdd 0.00 0.00 ~ (all equal) IntAdd-6 0.00 0.00 ~ (all equal) IntAdd-48 0.00 0.00 ~ (all equal) IntSet 0.00 0.00 ~ (all equal) IntSet-6 0.00 0.00 ~ (all equal) IntSet-48 0.00 0.00 ~ (all equal) FloatAdd 0.00 0.00 ~ (all equal) FloatAdd-6 0.00 0.00 ~ (all equal) FloatAdd-48 0.00 0.00 ~ (all equal) FloatSet 0.00 0.00 ~ (all equal) FloatSet-6 0.00 0.00 ~ (all equal) FloatSet-48 0.00 0.00 ~ (all equal) MapSet 0.00 3.00 ± 0% +Inf% (p=0.000 n=8+8) MapSet-6 0.00 3.00 ± 0% +Inf% (p=0.000 n=8+8) MapSet-48 0.00 3.00 ± 0% +Inf% (p=0.000 n=8+8) MapSetDifferent 0.00 12.00 ± 0% +Inf% (p=0.000 n=8+8) MapSetDifferent-6 0.00 12.00 ± 0% +Inf% (p=0.000 n=8+8) MapSetDifferent-48 0.00 12.00 ± 0% +Inf% (p=0.000 n=8+8) MapSetString 0.00 3.00 ± 0% +Inf% (p=0.000 n=8+8) MapSetString-6 0.00 3.00 ± 0% +Inf% (p=0.000 n=8+8) MapSetString-48 0.00 3.00 ± 0% +Inf% (p=0.000 n=8+8) MapAddSame 6.00 ± 0% 11.00 ± 0% +83.33% (p=0.000 n=8+8) MapAddSame-6 6.00 ± 0% 11.00 ± 0% +83.33% (p=0.000 n=8+8) MapAddSame-48 6.00 ± 0% 11.00 ± 0% +83.33% (p=0.000 n=8+8) MapAddDifferent 14.0 ± 0% 31.0 ± 0% +121.43% (p=0.000 n=8+8) MapAddDifferent-6 14.0 ± 0% 31.0 ± 0% +121.43% (p=0.000 n=8+8) MapAddDifferent-48 14.0 ± 0% 31.0 ± 0% +121.43% (p=0.000 n=8+8) MapAddSameSteadyState 0.00 0.00 ~ (all equal) MapAddSameSteadyState-6 0.00 0.00 ~ (all equal) MapAddSameSteadyState-48 0.00 0.00 ~ (all equal) MapAddDifferentSteadyState 0.00 0.00 ~ (all equal) MapAddDifferentSteadyState-6 0.00 0.00 ~ (all equal) MapAddDifferentSteadyState-48 0.00 0.00 ~ (all equal) RealworldExpvarUsage 0.00 0.00 ~ (all equal) RealworldExpvarUsage-6 0.00 0.00 ~ (all equal) RealworldExpvarUsage-48 0.00 0.00 ~ (all equal) https://perf.golang.org/search?q=upload:20170427.1 Change-Id: I388b2e8a3cadb84fc1418af8acfc27338f799273 Reviewed-on: https://go-review.googlesource.com/41930 Run-TryBot: Bryan Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 95e7897 commit fb0fe42

File tree

2 files changed

+63
-93
lines changed

2 files changed

+63
-93
lines changed

src/expvar/expvar.go

Lines changed: 52 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ func (v *Float) Set(value float64) {
9999

100100
// Map is a string-to-Var map variable that satisfies the Var interface.
101101
type Map struct {
102-
mu sync.RWMutex
103-
m map[string]Var
104-
keys []string // sorted
102+
m sync.Map // map[string]Var
103+
keysMu sync.RWMutex
104+
keys []string // sorted
105105
}
106106

107107
// KeyValue represents a single entry in a Map.
@@ -111,12 +111,10 @@ type KeyValue struct {
111111
}
112112

113113
func (v *Map) String() string {
114-
v.mu.RLock()
115-
defer v.mu.RUnlock()
116114
var b bytes.Buffer
117115
fmt.Fprintf(&b, "{")
118116
first := true
119-
v.doLocked(func(kv KeyValue) {
117+
v.Do(func(kv KeyValue) {
120118
if !first {
121119
fmt.Fprintf(&b, ", ")
122120
}
@@ -127,79 +125,60 @@ func (v *Map) String() string {
127125
return b.String()
128126
}
129127

130-
func (v *Map) Init() *Map {
131-
v.m = make(map[string]Var)
132-
return v
133-
}
128+
func (v *Map) Init() *Map { return v }
134129

135130
// updateKeys updates the sorted list of keys in v.keys.
136-
// must be called with v.mu held.
137-
func (v *Map) updateKeys() {
138-
if len(v.m) == len(v.keys) {
139-
// No new key.
140-
return
141-
}
142-
v.keys = v.keys[:0]
143-
for k := range v.m {
144-
v.keys = append(v.keys, k)
145-
}
131+
func (v *Map) addKey(key string) {
132+
v.keysMu.Lock()
133+
defer v.keysMu.Unlock()
134+
v.keys = append(v.keys, key)
146135
sort.Strings(v.keys)
147136
}
148137

149138
func (v *Map) Get(key string) Var {
150-
v.mu.RLock()
151-
defer v.mu.RUnlock()
152-
return v.m[key]
139+
i, _ := v.m.Load(key)
140+
av, _ := i.(Var)
141+
return av
153142
}
154143

155144
func (v *Map) Set(key string, av Var) {
156-
v.mu.Lock()
157-
defer v.mu.Unlock()
158-
v.m[key] = av
159-
v.updateKeys()
145+
if _, dup := v.m.LoadOrStore(key, av); dup {
146+
v.m.Store(key, av)
147+
} else {
148+
v.addKey(key)
149+
}
160150
}
161151

152+
// Add adds delta to the *Int value stored under the given map key.
162153
func (v *Map) Add(key string, delta int64) {
163-
v.mu.RLock()
164-
av, ok := v.m[key]
165-
v.mu.RUnlock()
154+
i, ok := v.m.Load(key)
166155
if !ok {
167-
// check again under the write lock
168-
v.mu.Lock()
169-
av, ok = v.m[key]
170-
if !ok {
171-
av = new(Int)
172-
v.m[key] = av
173-
v.updateKeys()
156+
var dup bool
157+
i, dup = v.m.LoadOrStore(key, new(Int))
158+
if !dup {
159+
v.addKey(key)
174160
}
175-
v.mu.Unlock()
176161
}
177162

178163
// Add to Int; ignore otherwise.
179-
if iv, ok := av.(*Int); ok {
164+
if iv, ok := i.(*Int); ok {
180165
iv.Add(delta)
181166
}
182167
}
183168

184169
// AddFloat adds delta to the *Float value stored under the given map key.
185170
func (v *Map) AddFloat(key string, delta float64) {
186-
v.mu.RLock()
187-
av, ok := v.m[key]
188-
v.mu.RUnlock()
171+
i, ok := v.m.Load(key)
189172
if !ok {
190-
// check again under the write lock
191-
v.mu.Lock()
192-
av, ok = v.m[key]
193-
if !ok {
194-
av = new(Float)
195-
v.m[key] = av
196-
v.updateKeys()
173+
var dup bool
174+
i, dup = v.m.LoadOrStore(key, new(Float))
175+
if !dup {
176+
v.addKey(key)
197177
}
198-
v.mu.Unlock()
199178
}
200179

201180
// Add to Float; ignore otherwise.
202-
if iv, ok := av.(*Float); ok {
181+
if iv, ok := i.(*Float); ok {
203182
iv.Add(delta)
204183
}
205184
}
@@ -208,45 +187,34 @@ func (v *Map) AddFloat(key string, delta float64) {
208187
// The map is locked during the iteration,
209188
// but existing entries may be concurrently updated.
210189
func (v *Map) Do(f func(KeyValue)) {
211-
v.mu.RLock()
212-
defer v.mu.RUnlock()
213-
v.doLocked(f)
214-
}
215-
216-
// doLocked calls f for each entry in the map.
217-
// v.mu must be held for reads.
218-
func (v *Map) doLocked(f func(KeyValue)) {
190+
v.keysMu.RLock()
191+
defer v.keysMu.RUnlock()
219192
for _, k := range v.keys {
220-
f(KeyValue{k, v.m[k]})
193+
i, _ := v.m.Load(k)
194+
f(KeyValue{k, i.(Var)})
221195
}
222196
}
223197

224198
// String is a string variable, and satisfies the Var interface.
225199
type String struct {
226-
mu sync.RWMutex
227-
s string
200+
s atomic.Value // string
228201
}
229202

230203
func (v *String) Value() string {
231-
v.mu.RLock()
232-
defer v.mu.RUnlock()
233-
return v.s
204+
p, _ := v.s.Load().(string)
205+
return p
234206
}
235207

236208
// String implements the Val interface. To get the unquoted string
237209
// use Value.
238210
func (v *String) String() string {
239-
v.mu.RLock()
240-
s := v.s
241-
v.mu.RUnlock()
211+
s := v.Value()
242212
b, _ := json.Marshal(s)
243213
return string(b)
244214
}
245215

246216
func (v *String) Set(value string) {
247-
v.mu.Lock()
248-
defer v.mu.Unlock()
249-
v.s = value
217+
v.s.Store(value)
250218
}
251219

252220
// Func implements Var by calling the function
@@ -264,31 +232,30 @@ func (f Func) String() string {
264232

265233
// All published variables.
266234
var (
267-
mutex sync.RWMutex
268-
vars = make(map[string]Var)
269-
varKeys []string // sorted
235+
vars sync.Map // map[string]Var
236+
varKeysMu sync.RWMutex
237+
varKeys []string // sorted
270238
)
271239

272240
// Publish declares a named exported variable. This should be called from a
273241
// package's init function when it creates its Vars. If the name is already
274242
// registered then this will log.Panic.
275243
func Publish(name string, v Var) {
276-
mutex.Lock()
277-
defer mutex.Unlock()
278-
if _, existing := vars[name]; existing {
244+
if _, dup := vars.LoadOrStore(name, v); dup {
279245
log.Panicln("Reuse of exported var name:", name)
280246
}
281-
vars[name] = v
247+
varKeysMu.Lock()
248+
defer varKeysMu.Unlock()
282249
varKeys = append(varKeys, name)
283250
sort.Strings(varKeys)
284251
}
285252

286253
// Get retrieves a named exported variable. It returns nil if the name has
287254
// not been registered.
288255
func Get(name string) Var {
289-
mutex.RLock()
290-
defer mutex.RUnlock()
291-
return vars[name]
256+
i, _ := vars.Load(name)
257+
v, _ := i.(Var)
258+
return v
292259
}
293260

294261
// Convenience functions for creating new exported variables.
@@ -321,10 +288,11 @@ func NewString(name string) *String {
321288
// The global variable map is locked during the iteration,
322289
// but existing entries may be concurrently updated.
323290
func Do(f func(KeyValue)) {
324-
mutex.RLock()
325-
defer mutex.RUnlock()
291+
varKeysMu.RLock()
292+
defer varKeysMu.RUnlock()
326293
for _, k := range varKeys {
327-
f(KeyValue{k, vars[k]})
294+
val, _ := vars.Load(k)
295+
f(KeyValue{k, val.(Var)})
328296
}
329297
}
330298

src/expvar/expvar_test.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ import (
2121
// RemoveAll removes all exported variables.
2222
// This is for tests only.
2323
func RemoveAll() {
24-
mutex.Lock()
25-
defer mutex.Unlock()
26-
vars = make(map[string]Var)
24+
varKeysMu.Lock()
25+
defer varKeysMu.Unlock()
26+
for _, k := range varKeys {
27+
vars.Delete(k)
28+
}
2729
varKeys = nil
2830
}
2931

@@ -130,22 +132,22 @@ func BenchmarkFloatSet(b *testing.B) {
130132
func TestString(t *testing.T) {
131133
RemoveAll()
132134
name := NewString("my-name")
133-
if name.Value() != "" {
134-
t.Errorf("name.Value() = %q, want \"\"", name.s)
135+
if s := name.Value(); s != "" {
136+
t.Errorf(`NewString("my-name").Value() = %q, want ""`, s)
135137
}
136138

137139
name.Set("Mike")
138140
if s, want := name.String(), `"Mike"`; s != want {
139-
t.Errorf("from %q, name.String() = %q, want %q", name.s, s, want)
141+
t.Errorf(`after name.Set("Mike"), name.String() = %q, want %q`, s, want)
140142
}
141143
if s, want := name.Value(), "Mike"; s != want {
142-
t.Errorf("from %q, name.Value() = %q, want %q", name.s, s, want)
144+
t.Errorf(`after name.Set("Mike"), name.Value() = %q, want %q`, s, want)
143145
}
144146

145147
// Make sure we produce safe JSON output.
146-
name.Set(`<`)
148+
name.Set("<")
147149
if s, want := name.String(), "\"\\u003c\""; s != want {
148-
t.Errorf("from %q, name.String() = %q, want %q", name.s, s, want)
150+
t.Errorf(`after name.Set("<"), name.String() = %q, want %q`, s, want)
149151
}
150152
}
151153

0 commit comments

Comments
 (0)