Skip to content

Commit

Permalink
Add zed.Value.Under parameter to avoid allocation (#4740)
Browse files Browse the repository at this point in the history
Value.Under allocates memory on any call that does not return the
receiver.  This becomes spendy in runtime/expr.DotExpr.Eval on workloads
that access fields of records with named types.  Add a parameter to
Under that, when not nil, points to a Value to use in lieu of
allocating.

goos: darwin
goarch: arm64
pkg: github.com/brimdata/zed
                        |  before.txt  |              after.txt               |
                        |    sec/op    |    sec/op     vs base                |
ValueUnder/primitive-10   0.9348n ± 0%   0.9331n ± 1%        ~ (p=0.617 n=10)
ValueUnder/named-10       27.480n ± 0%    6.385n ± 1%  -76.76% (p=0.000 n=10)
geomean                    5.068n         2.441n       -51.84%
  • Loading branch information
nwt authored Aug 10, 2023
1 parent b8e1b18 commit 2689e24
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 14 deletions.
2 changes: 1 addition & 1 deletion runtime/expr/agg/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,5 @@ func valueUnder(typ zed.Type, b zcode.Bytes) *zed.Value {
if _, ok := zed.TypeUnder(typ).(*zed.TypeUnion); !ok {
return val
}
return val.Under()
return val.Under(val)
}
3 changes: 2 additions & 1 deletion runtime/expr/dot.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ func NewDottedExpr(zctx *zed.Context, f field.Path) Evaluator {
}

func (d *DotExpr) Eval(ectx Context, this *zed.Value) *zed.Value {
val := d.record.Eval(ectx, this).Under()
var tmpVal zed.Value
val := d.record.Eval(ectx, this).Under(&tmpVal)
// Cases are ordered by decreasing expected frequency.
switch typ := val.Type.(type) {
case *zed.TypeRecord:
Expand Down
7 changes: 5 additions & 2 deletions runtime/op/join/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type Op struct {
cutter *expr.Cutter
joinKey *zed.Value
joinSet []*zed.Value
tmpLeft zed.Value
types map[int]map[int]*zed.TypeRecord
}

Expand Down Expand Up @@ -238,8 +239,10 @@ func (o *Op) splice(ectx *expr.ResetContext, left, right *zed.Value) (*zed.Value
// stream.
return left, nil
}
left = left.Under()
right = right.Under()
// tmpLeft lives in Op because the 1.20.6 compiler thinks it escapes.
left = left.Under(&o.tmpLeft)
var tmpRight zed.Value
right = right.Under(&tmpRight)
typ, err := o.combinedType(zed.TypeRecordOf(left.Type), zed.TypeRecordOf(right.Type))
if err != nil {
return nil, err
Expand Down
5 changes: 3 additions & 2 deletions runtime/op/traverse/over.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,15 @@ func (o *Over) over(batch zbuf.Batch, this *zed.Value) zbuf.Batch {
}

func appendOver(zctx *zed.Context, vals []zed.Value, val zed.Value) []zed.Value {
val = *val.Under()
val = *val.Under(&val)
switch typ := zed.TypeUnder(val.Type).(type) {
case *zed.TypeArray, *zed.TypeSet:
typ = zed.InnerType(typ)
for it := val.Bytes().Iter(); !it.Done(); {
// XXX when we do proper expr.Context, we can allocate
// this copy through the batch.
val := zed.NewValue(typ, it.Next()).Under()
val := zed.NewValue(typ, it.Next())
val = val.Under(val)
vals = append(vals, *val.Copy())
}
return vals
Expand Down
16 changes: 11 additions & 5 deletions value.go
Original file line number Diff line number Diff line change
Expand Up @@ -438,24 +438,30 @@ func (v *Value) MissingAsNull() *Value {
}

// Under resolves named types and untags unions repeatedly, returning a value
// guaranteed to have neither a named type nor a union type.
func (v *Value) Under() *Value {
// guaranteed to have neither a named type nor a union type. When Under returns
// a new value (i.e., one that differs from v), it uses dst if not nil.
// Otherwise, Under allocates a new value.
func (v *Value) Under(dst *Value) *Value {
switch v.Type.(type) {
case *TypeUnion, *TypeNamed:
return v.under()
return v.under(dst)
}
// This is the common case; make sure the compiler can inline it.
return v
}

// under contains logic for Under that the compiler won't inline.
func (v *Value) under() *Value {
func (v *Value) under(dst *Value) *Value {
typ, bytes := v.Type, v.Bytes()
for {
typ = TypeUnder(typ)
union, ok := typ.(*TypeUnion)
if !ok {
return &Value{typ, bytes}
if dst == nil {
return &Value{typ, bytes}
}
*dst = Value{typ, bytes}
return dst
}
typ, bytes = union.Untag(bytes)
}
Expand Down
5 changes: 3 additions & 2 deletions value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,20 @@ import (
)

func BenchmarkValueUnder(b *testing.B) {
var tmpVal zed.Value
b.Run("primitive", func(b *testing.B) {
val := zed.Null
b.ResetTimer()
for i := 0; i < b.N; i++ {
val.Under()
val.Under(&tmpVal)
}
})
b.Run("named", func(b *testing.B) {
typ, _ := zed.NewContext().LookupTypeNamed("name", zed.TypeNull)
val := zed.NewValue(typ, nil)
b.ResetTimer()
for i := 0; i < b.N; i++ {
val.Under()
val.Under(&tmpVal)
}
})
}
Expand Down
3 changes: 2 additions & 1 deletion zio/csvio/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ func (w *Writer) Write(rec *zed.Value) error {
for i, it := 0, rec.Bytes().Iter(); i < len(fields) && !it.Done(); i++ {
var s string
if zb := it.Next(); zb != nil {
val := zed.NewValue(fields[i].Type, zb).Under()
val := zed.NewValue(fields[i].Type, zb)
val = val.Under(val)
switch id := val.Type.ID(); {
case id == zed.IDBytes && len(val.Bytes()) == 0:
// We want "" instead of "0x" for a zero-length value.
Expand Down

0 comments on commit 2689e24

Please sign in to comment.