From 2689e242f9810e0e4c11f8f132963da776395dcb Mon Sep 17 00:00:00 2001 From: Noah Treuhaft Date: Thu, 10 Aug 2023 18:04:31 -0400 Subject: [PATCH] Add zed.Value.Under parameter to avoid allocation (#4740) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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% --- runtime/expr/agg/map.go | 2 +- runtime/expr/dot.go | 3 ++- runtime/op/join/join.go | 7 +++++-- runtime/op/traverse/over.go | 5 +++-- value.go | 16 +++++++++++----- value_test.go | 5 +++-- zio/csvio/writer.go | 3 ++- 7 files changed, 27 insertions(+), 14 deletions(-) diff --git a/runtime/expr/agg/map.go b/runtime/expr/agg/map.go index 874804246a..a4440e8e5c 100644 --- a/runtime/expr/agg/map.go +++ b/runtime/expr/agg/map.go @@ -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) } diff --git a/runtime/expr/dot.go b/runtime/expr/dot.go index d0b6fe4bc3..a0e1fae9f4 100644 --- a/runtime/expr/dot.go +++ b/runtime/expr/dot.go @@ -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: diff --git a/runtime/op/join/join.go b/runtime/op/join/join.go index 72808099b1..315ac8eff8 100644 --- a/runtime/op/join/join.go +++ b/runtime/op/join/join.go @@ -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 } @@ -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 diff --git a/runtime/op/traverse/over.go b/runtime/op/traverse/over.go index 8d29ca466e..929a41b2f2 100644 --- a/runtime/op/traverse/over.go +++ b/runtime/op/traverse/over.go @@ -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 diff --git a/value.go b/value.go index 8668a60c1b..23dcb11943 100644 --- a/value.go +++ b/value.go @@ -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) } diff --git a/value_test.go b/value_test.go index 4908c8de7c..7292b7fa6f 100644 --- a/value_test.go +++ b/value_test.go @@ -9,11 +9,12 @@ 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) { @@ -21,7 +22,7 @@ func BenchmarkValueUnder(b *testing.B) { val := zed.NewValue(typ, nil) b.ResetTimer() for i := 0; i < b.N; i++ { - val.Under() + val.Under(&tmpVal) } }) } diff --git a/zio/csvio/writer.go b/zio/csvio/writer.go index cac1d94b92..56a9a50c19 100644 --- a/zio/csvio/writer.go +++ b/zio/csvio/writer.go @@ -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.