Skip to content

Commit b8497fa

Browse files
authored
Dynamic config conversion improvements (#7052)
## What changed? - Split implementation of "constrained default" settings from "plain default" settings. This is more code and the diff looks complex, but the individual paths are both simpler than the mixed version. - Add conversion cache using a weak map. - Remove GlobalCachedTypedValue. - Use "raw" values for subscription dispatch deduping to avoid unnecessary conversions. - Deep copy default values when using mapstructure, to avoid problems with merging over shared default values. ## Why? - Fixes #6756 - Performance improvement for "plain default" settings (almost all of them) - Performance improvement for settings with complex converters - Remove footgun in defaults that aren't scalar values ## How did you test it? existing+new unit tests
1 parent f9bd083 commit b8497fa

File tree

20 files changed

+935
-351
lines changed

20 files changed

+935
-351
lines changed

cmd/tools/gendynamicconfig/dynamic_config.tmpl

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@ const (
2222
{{- range $P := $Precedences}}
2323
{{ if $T.IsGeneric -}}
2424
type {{$P.Name}}TypedSetting[T any] setting[T, func({{$P.GoArgs}})]
25+
type {{$P.Name}}TypedConstrainedDefaultSetting[T any] constrainedDefaultSetting[T, func({{$P.GoArgs}})]
2526

2627
// New{{$P.Name}}TypedSetting creates a setting that uses mapstructure to handle complex structured
27-
// values. The value from dynamic config will be copied over a shallow copy of 'def', which means
28-
// 'def' must not contain any non-nil slices, maps, or pointers.
28+
// values. The value from dynamic config will be _merged_ over a deep copy of 'def'. Be very careful
29+
// when using non-empty maps or slices as defaults, the result may not be what you want.
2930
func New{{$P.Name}}TypedSetting[T any](key Key, def T, description string) {{$P.Name}}TypedSetting[T] {
3031
s := {{$P.Name}}TypedSetting[T]{
3132
key: key,
@@ -50,10 +51,10 @@ func New{{$P.Name}}TypedSettingWithConverter[T any](key Key, convert func(any) (
5051
}
5152

5253
// New{{$P.Name}}TypedSettingWithConstrainedDefault creates a setting with a compound default value.
53-
func New{{$P.Name}}TypedSettingWithConstrainedDefault[T any](key Key, convert func(any) (T, error), cdef []TypedConstrainedValue[T], description string) {{$P.Name}}TypedSetting[T] {
54-
s := {{$P.Name}}TypedSetting[T]{
54+
func New{{$P.Name}}TypedSettingWithConstrainedDefault[T any](key Key, convert func(any) (T, error), cdef []TypedConstrainedValue[T], description string) {{$P.Name}}TypedConstrainedDefaultSetting[T] {
55+
s := {{$P.Name}}TypedConstrainedDefaultSetting[T]{
5556
key: key,
56-
cdef: &cdef,
57+
cdef: cdef,
5758
convert: convert,
5859
description: description,
5960
}
@@ -68,6 +69,13 @@ func (s {{$P.Name}}TypedSetting[T]) Validate(v any) error {
6869
return err
6970
}
7071

72+
func (s {{$P.Name}}TypedConstrainedDefaultSetting[T]) Key() Key { return s.key }
73+
func (s {{$P.Name}}TypedConstrainedDefaultSetting[T]) Precedence() Precedence { return Precedence{{$P.Name}} }
74+
func (s {{$P.Name}}TypedConstrainedDefaultSetting[T]) Validate(v any) error {
75+
_, err := s.convert(v)
76+
return err
77+
}
78+
7179
func (s {{$P.Name}}TypedSetting[T]) WithDefault(v T) {{$P.Name}}TypedSetting[T] {
7280
newS := s
7381
newS.def = v
@@ -92,6 +100,22 @@ func (s {{$P.Name}}TypedSetting[T]) Get(c *Collection) TypedPropertyFnWith{{$P.N
92100
c,
93101
s.key,
94102
s.def,
103+
s.convert,
104+
prec,
105+
)
106+
}
107+
}
108+
109+
{{if eq $P.Name "Global" -}}
110+
func (s {{$P.Name}}TypedConstrainedDefaultSetting[T]) Get(c *Collection) TypedPropertyFn[T] {
111+
{{- else -}}
112+
func (s {{$P.Name}}TypedConstrainedDefaultSetting[T]) Get(c *Collection) TypedPropertyFnWith{{$P.Name}}Filter[T] {
113+
{{- end}}
114+
return func({{$P.GoArgs}}) T {
115+
prec := {{$P.Expr}}
116+
return matchAndConvertWithConstrainedDefault(
117+
c,
118+
s.key,
95119
s.cdef,
96120
s.convert,
97121
prec,
@@ -113,7 +137,7 @@ func (s {{$P.Name}}TypedSetting[T]) Subscribe(c *Collection) TypedSubscribableWi
113137
return func({{$P.GoArgs}}, callback func(T)) (T, func()) {
114138
{{- end}}
115139
prec := {{$P.Expr}}
116-
return subscribe(c, s.key, s.def, s.cdef, s.convert, prec, callback)
140+
return subscribe(c, s.key, s.def, s.convert, prec, callback)
117141
}
118142
}
119143

@@ -127,6 +151,10 @@ func (s {{$P.Name}}TypedSetting[T]) dispatchUpdate(c *Collection, sub any, cvs [
127151
)
128152
}
129153

154+
func (s {{$P.Name}}TypedConstrainedDefaultSetting[T]) dispatchUpdate(c *Collection, sub any, cvs []ConstrainedValue) {
155+
// can't subscribe to constrained default settings
156+
}
157+
130158
{{if eq $P.Name "Global" -}}
131159
func GetTypedPropertyFn[T any](value T) TypedPropertyFn[T] {
132160
{{- else -}}
@@ -138,12 +166,13 @@ func GetTypedPropertyFnFilteredBy{{$P.Name}}[T any](value T) TypedPropertyFnWith
138166
}
139167
{{else -}}
140168
type {{$P.Name}}{{$T.Name}}Setting = {{$P.Name}}TypedSetting[{{$T.GoType}}]
169+
type {{$P.Name}}{{$T.Name}}ConstrainedDefaultSetting = {{$P.Name}}TypedConstrainedDefaultSetting[{{$T.GoType}}]
141170

142171
func New{{$P.Name}}{{$T.Name}}Setting(key Key, def {{$T.GoType}}, description string) {{$P.Name}}{{$T.Name}}Setting {
143172
return New{{$P.Name}}TypedSettingWithConverter[{{$T.GoType}}](key, convert{{$T.Name}}, def, description)
144173
}
145174

146-
func New{{$P.Name}}{{$T.Name}}SettingWithConstrainedDefault(key Key, cdef []TypedConstrainedValue[{{$T.GoType}}], description string) {{$P.Name}}{{$T.Name}}Setting {
175+
func New{{$P.Name}}{{$T.Name}}SettingWithConstrainedDefault(key Key, cdef []TypedConstrainedValue[{{$T.GoType}}], description string) {{$P.Name}}{{$T.Name}}ConstrainedDefaultSetting {
147176
return New{{$P.Name}}TypedSettingWithConstrainedDefault[{{$T.GoType}}](key, convert{{$T.Name}}, cdef, description)
148177
}
149178

@@ -164,4 +193,4 @@ func Get{{$T.Name}}PropertyFnFilteredBy{{$P.Name}}(value {{$T.GoType}}) {{$T.Nam
164193
{{- end}}
165194
{{end }}{{/* if $T.IsGeneric */}}
166195
{{- end}}{{/* range $T :=.Types */}}
167-
{{- end}}{{/* range $P := $Precedences */}}
196+
{{- end}}{{/* range $P := $Precedences */}}

common/dynamicconfig/cachedvalue.go

Lines changed: 0 additions & 58 deletions
This file was deleted.

common/dynamicconfig/client.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ type (
2323
// Note that GetValue is called very often! You should not synchronously call out to an
2424
// external system. Instead you should keep a set of all configured values, refresh it
2525
// periodically or when notified, and only do in-memory lookups inside of GetValue.
26+
//
27+
// Implementations should prefer to return the same slice in response to the same key
28+
// as long as the value hasn't changed. Value conversions are cached using weak
29+
// pointers into the returned slice, so new slices will result in unnecessary calls to
30+
// conversion functions.
2631
GetValue(key Key) []ConstrainedValue
2732
}
2833

0 commit comments

Comments
 (0)