this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

internal/core/adt: avoid allocations in matchPattern

Feature.ToValue was called eagerly for every string label before pattern
matching, allocating a *String even though the fast path only needs the
string content. This change retrieves the string directly via
ctx.IndexToString for fast-path cases, only allocating in the slow path.

This also fixes a bug in jsonschema where error patterns were not
handled correctly. The old code checked `!k.IsAnyOf(pattern.Kind())`
before the switch statement, but *Bottom has Kind() == BottomKind (0),
so this check always failed for error patterns, returning false
before the *Bottom case could be reached. The new code handles *Bottom
in the switch before any Kind filtering occurs.

The bug fix cannot be easily separated from the optimization because
both changes affect how pattern matching flows through the code, and
attempts to isolate the fix while keeping the old allocation behavior
were unsuccessful.

│ old │ new │
│ B/op │ B/op vs base │
VetInventory 4.592Gi ± ∞ ¹ 4.565Gi ± ∞ ¹ -0.58% (p=1.000 n=1)

│ old │ new │
│ allocs/op │ allocs/op vs base │
VetInventory 47.89M ± ∞ ¹ 47.29M ± ∞ ¹ -1.26% (p=1.000 n=1)

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I2c815823ff2412e32f1fbc967c847bed3101291b
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1229642
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Matthew Sackman <matthew@cue.works>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>

+17 -45
+2 -2
encoding/jsonschema/external_teststats.txt
··· 4 4 5 5 v3: 6 6 schema extract (pass / total): 1072 / 1363 = 78.7% 7 - tests (pass / total): 3913 / 4803 = 81.5% 8 - tests on extracted schemas (pass / total): 3913 / 4041 = 96.8% 7 + tests (pass / total): 3917 / 4803 = 81.6% 8 + tests on extracted schemas (pass / total): 3917 / 4041 = 96.9% 9 9 10 10 v3-roundtrip: 11 11 schema extract (pass / total): 240 / 1363 = 17.6%
-1
encoding/jsonschema/testdata/external/tests/draft2019-09/propertyNames.json
··· 171 171 }, 172 172 "valid": false, 173 173 "skip": { 174 - "v3": "unexpected success", 175 174 "v3-roundtrip": "could not extract schema" 176 175 } 177 176 },
-1
encoding/jsonschema/testdata/external/tests/draft2020-12/propertyNames.json
··· 114 114 }, 115 115 "valid": false, 116 116 "skip": { 117 - "v3": "unexpected success", 118 117 "v3-roundtrip": "unexpected success" 119 118 } 120 119 },
-1
encoding/jsonschema/testdata/external/tests/draft6/propertyNames.json
··· 167 167 }, 168 168 "valid": false, 169 169 "skip": { 170 - "v3": "unexpected success", 171 170 "v3-roundtrip": "could not extract schema" 172 171 } 173 172 },
-1
encoding/jsonschema/testdata/external/tests/draft7/propertyNames.json
··· 167 167 }, 168 168 "valid": false, 169 169 "skip": { 170 - "v3": "unexpected success", 171 170 "v3-roundtrip": "could not extract schema" 172 171 } 173 172 },
+11 -31
internal/core/adt/constraints.go
··· 127 127 return false 128 128 } 129 129 130 - // TODO(perf): this assumes that comparing an int64 against apd.Decimal 131 - // is faster than converting this to a Num and using that for comparison. 132 - // This may very well not be the case. But it definitely will be if we 133 - // special-case integers that can fit in an int64 (or int32 if we want to 134 - // avoid many bound checks), which we probably should. Especially when we 135 - // allow list constraints, like [<10]: T. 136 - var label Value 137 - if f.IsString() && int64(f.Index()) != MaxIndex { 138 - label = f.ToValue(ctx) 139 - } 140 - 141 - return matchPatternValue(ctx, pattern, f, label) 130 + return matchPatternValue(ctx, pattern, f) 142 131 } 143 132 144 - // matchPatternValue matches a concrete value against f. label must be the 145 - // CUE value that is obtained from converting f. 133 + // matchPatternValue matches a concrete value against f. 146 134 // 147 135 // This is an optimization an intended to be faster than regular CUE evaluation 148 136 // for the majority of cases where pattern constraints are used. 149 - func matchPatternValue(ctx *OpContext, pattern Value, f Feature, label Value) (result bool) { 137 + func matchPatternValue(ctx *OpContext, pattern Value, f Feature) (result bool) { 150 138 if v, ok := pattern.(*Vertex); ok { 151 139 v.unify(ctx, Flags{condition: scalarKnown, mode: finalize, checkTypos: false}) 152 140 } 153 141 pattern = Unwrap(pattern) 154 - label = Unwrap(label) 155 - 156 - if pattern == label { 157 - return true 158 - } 159 142 160 143 k := IntKind 161 144 if f.IsString() { ··· 193 176 case *BoundValue: 194 177 switch x.Kind() { 195 178 case StringKind: 196 - if label == nil { 179 + if !f.IsString() || int64(f.Index()) == MaxIndex { 197 180 return false 198 181 } 199 - str := label.(*String).Str 182 + str := ctx.IndexToString(f.safeIndex()) 200 183 return x.validateStr(ctx, str) 201 184 202 185 case NumberKind: ··· 212 195 return err == nil && xi == yi 213 196 214 197 case *String: 215 - if label == nil { 198 + if !f.IsString() || int64(f.Index()) == MaxIndex { 216 199 return false 217 200 } 218 - y, ok := label.(*String) 219 - return ok && x.Str == y.Str 201 + str := ctx.IndexToString(f.safeIndex()) 202 + return x.Str == str 220 203 221 204 case *Conjunction: 222 205 for _, a := range x.Values { 223 - if !matchPatternValue(ctx, a, f, label) { 206 + if !matchPatternValue(ctx, a, f) { 224 207 return false 225 208 } 226 209 } ··· 228 211 229 212 case *Disjunction: 230 213 for _, a := range x.Values { 231 - if matchPatternValue(ctx, a, f, label) { 214 + if matchPatternValue(ctx, a, f) { 232 215 return true 233 216 } 234 217 } ··· 242 225 // slow track. One way to signal this would be to have a "value thunk" at 243 226 // the root that causes the fast track to be bypassed altogether. 244 227 245 - if label == nil { 246 - label = f.ToValue(ctx) 247 - } 248 - 228 + label := f.ToValue(ctx) 249 229 n := ctx.newInlineVertex(nil, nil, 250 230 MakeConjunct(ctx.e, pattern, ctx.ci), 251 231 MakeConjunct(ctx.e, label, ctx.ci))
+2 -6
internal/core/adt/constraints_test.go
··· 135 135 expr := pv(t.T, tc.expr) 136 136 137 137 var f adt.Feature 138 - var label adt.Value 139 138 if tc.label != "" { 140 - f, label = str(tc.label) 139 + f, _ = str(tc.label) 141 140 } else { 142 141 f = idx(tc.index) 143 142 } 144 - if tc.value != "" { 145 - label = pv(t.T, tc.value) 146 - } 147 143 148 - t.Equal(adt.MatchPatternValue(ctx, expr, f, label), tc.result) 144 + t.Equal(adt.MatchPatternValue(ctx, expr, f), tc.result) 149 145 }) 150 146 }
+2 -2
internal/core/adt/export_test.go
··· 18 18 // fields_test.go and constraints_test. 19 19 20 20 // MatchPatternValue exports matchPatternValue for testing. 21 - func MatchPatternValue(ctx *OpContext, p Value, f Feature, label Value) bool { 22 - return matchPatternValue(ctx, p, f, label) 21 + func MatchPatternValue(ctx *OpContext, p Value, f Feature) bool { 22 + return matchPatternValue(ctx, p, f) 23 23 }