Commit 9312355
committed
perf(sql): optimize expression evaluation (#4075)
## Summary
- Reuse the project evaluator and its valuer chain across input rows.
- Normalize numeric operands with one type switch instead of repeated
type checks and cast dispatch.
- Compare common `changed_cols` value types directly before falling back
to `reflect.DeepEqual`.
## Why
- SQL expression evaluation runs for every input row, so small setup and
dispatch costs accumulate in projection, arithmetic, filtering, sorting,
and change detection.
- The project operator rebuilt the same evaluator scaffolding for every
row even though only the current tuple, window, and aggregate data
changed.
- Numeric normalization inspected each operand type multiple times,
while `changed_cols` used reflection even for primitive values.
## Changes In This PR
- Expose reusable internal multi-valuer implementations and reset their
per-row data from `ProjectOp` without changing expression indices or
SliceTuple output semantics.
- Replace `convertNum` helper dispatch with a single type switch that
preserves the existing integer and floating-point conversions.
- Add primitive fast paths for string, int64, float64, bool, and int
comparisons in `changed_cols`; complex values retain `reflect.DeepEqual`
behavior.
- Add focused regression tests and benchmarks for all three paths.
- Organize the changes as three independent commits for review.
## Notes
- Benchmark medians on linux/amd64, Intel i9-14900HX, seven runs:
- int64 `SimpleDataEval`: 10.82 ns/op to 8.08 ns/op (about 25% faster),
0 allocations before and after.
- SliceTuple projection: 158.6 ns/op to 54.9 ns/op (about 65% faster),
from 112 B / 4 allocations to 0 B / 0 allocations.
- stable `changed_cols`: 337.0 ns/op to 304.8 ns/op (about 10% faster),
with allocations unchanged.
- Passed `go test -race ./internal/xsql ./internal/topo/operator
-count=1` and focused race tests for the changed column functions.
- `git diff --check` passes.
- A full local binder/function run still hits the pre-existing
`TestIncAggFunctionErr` failure, reproduced unchanged on
`upstream/master`. `go vet` likewise reports the existing lock-copy
warning in untouched `internal/xsql/row.go:187`.
---------
Signed-off-by: Jiayin Ng <ngjaying@gmail.com>1 parent 2ff8a9c commit 9312355
6 files changed
Lines changed: 264 additions & 53 deletions
File tree
- internal
- binder/function
- topo/operator
- xsql
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| |||
89 | 89 | | |
90 | 90 | | |
91 | 91 | | |
92 | | - | |
| 92 | + | |
93 | 93 | | |
94 | 94 | | |
95 | 95 | | |
| |||
102 | 102 | | |
103 | 103 | | |
104 | 104 | | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | | - | |
| 1 | + | |
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| |||
305 | 305 | | |
306 | 306 | | |
307 | 307 | | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
50 | 50 | | |
51 | 51 | | |
52 | 52 | | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
53 | 59 | | |
54 | 60 | | |
55 | 61 | | |
| |||
121 | 127 | | |
122 | 128 | | |
123 | 129 | | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
124 | 143 | | |
125 | | - | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
126 | 149 | | |
127 | 150 | | |
128 | | - | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
129 | 162 | | |
130 | | - | |
131 | 163 | | |
| 164 | + | |
132 | 165 | | |
133 | 166 | | |
134 | 167 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3630 | 3630 | | |
3631 | 3631 | | |
3632 | 3632 | | |
| 3633 | + | |
| 3634 | + | |
| 3635 | + | |
| 3636 | + | |
| 3637 | + | |
| 3638 | + | |
| 3639 | + | |
| 3640 | + | |
| 3641 | + | |
| 3642 | + | |
| 3643 | + | |
| 3644 | + | |
| 3645 | + | |
| 3646 | + | |
| 3647 | + | |
| 3648 | + | |
| 3649 | + | |
| 3650 | + | |
| 3651 | + | |
| 3652 | + | |
| 3653 | + | |
| 3654 | + | |
| 3655 | + | |
| 3656 | + | |
| 3657 | + | |
| 3658 | + | |
| 3659 | + | |
| 3660 | + | |
| 3661 | + | |
| 3662 | + | |
| 3663 | + | |
| 3664 | + | |
| 3665 | + | |
| 3666 | + | |
| 3667 | + | |
| 3668 | + | |
| 3669 | + | |
| 3670 | + | |
| 3671 | + | |
| 3672 | + | |
| 3673 | + | |
| 3674 | + | |
| 3675 | + | |
| 3676 | + | |
| 3677 | + | |
0 commit comments