Skip to content

Commit bc0ed02

Browse files
Fix the heap escape on cost-tracking with a fixed sized, static buffer (#1600)
1 parent 7e9441c commit bc0ed02

2 files changed

Lines changed: 63 additions & 8 deletions

File tree

‎common/cost/tracker.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@
1515
package cost
1616

1717
import (
18-
"cel.dev/cel-go/common/types/ref"
1918
"fmt"
19+
20+
"cel.dev/cel-go/common/types/ref"
2021
)
2122

2223
// WARNING: Any changes to cost calculations in this file require a corresponding change in estimator.go
@@ -129,6 +130,7 @@ type Tracker struct {
129130
cost uint64
130131
presenceTestHasCost bool
131132
modelVersion uint32
133+
argsBuf [2]ref.Val
132134
}
133135

134136
// NewTracker creates a new Tracker with a given estimator and a set of functional TrackerOption values.
@@ -231,17 +233,19 @@ func (c *Tracker) EvalZeroArity(vars any, id int64, call Call, result ref.Val) {
231233

232234
// EvalUnary records the cost for a unary call expression.
233235
func (c *Tracker) EvalUnary(vars any, id int64, call Call, arg ref.Val, result ref.Val) {
234-
var buf [1]ref.Val
235-
buf[0] = arg
236-
c.recordCallCost(call, buf[:], result)
236+
c.argsBuf[0] = arg
237+
c.argsBuf[1] = nil
238+
c.recordCallCost(call, c.argsBuf[:1], result)
239+
c.argsBuf[0] = nil
237240
}
238241

239242
// EvalBinary records the cost for a binary call expression.
240243
func (c *Tracker) EvalBinary(vars any, id int64, call Call, lhs, rhs ref.Val, result ref.Val) {
241-
var buf [2]ref.Val
242-
buf[0] = lhs
243-
buf[1] = rhs
244-
c.recordCallCost(call, buf[:], result)
244+
c.argsBuf[0] = lhs
245+
c.argsBuf[1] = rhs
246+
c.recordCallCost(call, c.argsBuf[:2], result)
247+
c.argsBuf[0] = nil
248+
c.argsBuf[1] = nil
245249
}
246250

247251
// EvalVarArgs records the cost for a variadic call expression.

‎common/cost/tracker_test.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,3 +404,54 @@ func TestTracker_ConcurrentCloneRace(t *testing.T) {
404404
}
405405
wg.Wait()
406406
}
407+
408+
type zeroAllocEstimator struct {
409+
cost uint64
410+
}
411+
412+
func (e *zeroAllocEstimator) CallCost(function, overloadID string, args []ref.Val, result ref.Val) *uint64 {
413+
return &e.cost
414+
}
415+
416+
func TestTracker_EvalUnaryBinaryAllocs(t *testing.T) {
417+
estimator := &zeroAllocEstimator{cost: 3}
418+
tracker, err := cost.NewTracker(estimator)
419+
if err != nil {
420+
t.Fatalf("NewTracker() failed: %v", err)
421+
}
422+
unmodeledTracker, err := cost.NewTracker(nil)
423+
if err != nil {
424+
t.Fatalf("NewTracker(nil) failed: %v", err)
425+
}
426+
427+
unaryCall := &testCall{function: "!", overloadID: overloads.LogicalNot}
428+
binaryCall := &testCall{function: "_+_", overloadID: overloads.AddInt64}
429+
arg1 := types.Int(1)
430+
arg2 := types.Int(2)
431+
res := types.Int(3)
432+
433+
if allocs := testing.AllocsPerRun(100, func() {
434+
unmodeledTracker.EvalUnary(nil, 1, unaryCall, types.True, types.False)
435+
}); allocs != 0 {
436+
t.Errorf("EvalUnary (unmodeled) allocated %v times, want 0", allocs)
437+
}
438+
439+
if allocs := testing.AllocsPerRun(100, func() {
440+
unmodeledTracker.EvalBinary(nil, 2, binaryCall, arg1, arg2, res)
441+
}); allocs != 0 {
442+
t.Errorf("EvalBinary (unmodeled) allocated %v times, want 0", allocs)
443+
}
444+
445+
if allocs := testing.AllocsPerRun(100, func() {
446+
tracker.EvalUnary(nil, 1, unaryCall, types.True, types.False)
447+
}); allocs != 0 {
448+
t.Errorf("EvalUnary (with estimator) allocated %v times, want 0", allocs)
449+
}
450+
451+
if allocs := testing.AllocsPerRun(100, func() {
452+
tracker.EvalBinary(nil, 2, binaryCall, arg1, arg2, res)
453+
}); allocs != 0 {
454+
t.Errorf("EvalBinary (with estimator) allocated %v times, want 0", allocs)
455+
}
456+
}
457+

0 commit comments

Comments
 (0)