Skip to content

Commit 16b1e29

Browse files
Avoid locking when converting dynamic config values (#8290)
## What changed? Uses a `sync.Map` instead of explicit locking for the dynamic config cache. ## Why? This is a very read-heavy map that is used under high concurrency. Protecting it with a single lock can cause poor performance due to contention. A `sync.Map` is designed to handle this usage pattern much better than a map protected by a lock. ## How did you test it? - [x] built - [ ] run locally and tested manually - [x] covered by existing tests - [ ] added new unit test(s) - [ ] added new functional test(s) ## Potential risks There is no change in functional behavior and no added risk of deadlocks or contention.
1 parent d2197f7 commit 16b1e29

File tree

1 file changed

+4
-14
lines changed

1 file changed

+4
-14
lines changed

common/dynamicconfig/collection.go

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@ type (
4444

4545
// cache converted values. use weak pointers to avoid holding on to values in the cache
4646
// that are no longer in use.
47-
convertCacheLock sync.Mutex
48-
convertCache map[weak.Pointer[ConstrainedValue]]any
47+
convertCache sync.Map // map[weak.Pointer[ConstrainedValue]]any
4948
}
5049

5150
subscription[T any] struct {
@@ -110,7 +109,6 @@ func NewCollection(client Client, logger log.Logger) *Collection {
110109
logger: logger,
111110
errCount: -1,
112111
subscriptions: make(map[Key]map[int]any),
113-
convertCache: make(map[weak.Pointer[ConstrainedValue]]any),
114112
}
115113
}
116114

@@ -522,33 +520,25 @@ func dispatchUpdateWithConstrainedDefault[T any](
522520
func convertWithCache[T any](c *Collection, key Key, convert func(any) (T, error), cvp *ConstrainedValue) (T, error) {
523521
weakcvp := weak.Make(cvp)
524522

525-
c.convertCacheLock.Lock()
526-
if converted, ok := c.convertCache[weakcvp]; ok {
523+
if converted, ok := c.convertCache.Load(weakcvp); ok {
527524
if t, ok := converted.(T); ok {
528-
c.convertCacheLock.Unlock()
529525
return t, nil
530526
}
531527
// Each key can only be used with a single type, so this shouldn't happen
532528
c.logger.Warn("Cached converted value has wrong type", tag.Key(key.String()))
533529
// Fall through to regular conversion
534530
}
535-
c.convertCacheLock.Unlock()
536531

537-
// unlock around convert
538532
t, err := convert(cvp.Value)
539533
if err != nil {
540534
var zero T
541535
return zero, err
542536
}
543537

544-
c.convertCacheLock.Lock()
545-
c.convertCache[weakcvp] = t
546-
c.convertCacheLock.Unlock()
538+
c.convertCache.Store(weakcvp, t)
547539

548540
runtime.AddCleanup(cvp, func(w weak.Pointer[ConstrainedValue]) {
549-
c.convertCacheLock.Lock()
550-
delete(c.convertCache, w)
551-
c.convertCacheLock.Unlock()
541+
c.convertCache.Delete(w)
552542
}, weakcvp)
553543

554544
return t, nil

0 commit comments

Comments
 (0)