From a8d72454a183a499b0bcdf6d97412fe33191e900 Mon Sep 17 00:00:00 2001 From: Martin Disibio Date: Tue, 24 Sep 2024 13:53:33 -0400 Subject: [PATCH] TraceQL performance improvements (#4114) * performance improvements * Exit binop early withou evaluating RHS if possible --- pkg/traceql/ast_execute.go | 18 +++- pkg/traceql/enum_statics.go | 1 + tempodb/encoding/vparquet4/block_traceql.go | 105 ++++++++++++-------- 3 files changed, 79 insertions(+), 45 deletions(-) diff --git a/pkg/traceql/ast_execute.go b/pkg/traceql/ast_execute.go index 6f2bc000362..464c0264109 100644 --- a/pkg/traceql/ast_execute.go +++ b/pkg/traceql/ast_execute.go @@ -330,6 +330,20 @@ func (o *BinaryOperation) execute(span Span) (Static, error) { return NewStaticNil(), err } + // Look for cases where we don't even need to evalulate the RHS + if lhsB, ok := lhs.Bool(); ok { + if o.Op == OpAnd && !lhsB { + // x && y + // x is false so we don't need to evalulate y + return StaticFalse, nil + } + if o.Op == OpOr && lhsB { + // x || y + // x is true so we don't need to evalulate y + return StaticTrue, nil + } + } + rhs, err := o.RHS.execute(span) if err != nil { return NewStaticNil(), err @@ -339,7 +353,7 @@ func (o *BinaryOperation) execute(span Span) (Static, error) { lhsT := lhs.Type rhsT := rhs.Type if !lhsT.isMatchingOperand(rhsT) { - return NewStaticBool(false), nil + return StaticFalse, nil } if !o.Op.binaryTypesValid(lhsT, rhsT) { @@ -585,7 +599,7 @@ func (a Attribute) execute(span Span) (Static, error) { return static, nil } - return NewStaticNil(), nil + return StaticNil, nil } func uniqueSpans(ss1 []*Spanset, ss2 []*Spanset) []Span { diff --git a/pkg/traceql/enum_statics.go b/pkg/traceql/enum_statics.go index c1c6050720c..2e161adfd23 100644 --- a/pkg/traceql/enum_statics.go +++ b/pkg/traceql/enum_statics.go @@ -174,6 +174,7 @@ func (k Kind) String() string { } var ( + StaticNil = NewStaticNil() StaticTrue = NewStaticBool(true) StaticFalse = NewStaticBool(false) ) diff --git a/tempodb/encoding/vparquet4/block_traceql.go b/tempodb/encoding/vparquet4/block_traceql.go index d786535aaab..f67fc272e36 100644 --- a/tempodb/encoding/vparquet4/block_traceql.go +++ b/tempodb/encoding/vparquet4/block_traceql.go @@ -122,6 +122,7 @@ func (s *span) AttributeFor(a traceql.Attribute) (traceql.Static, bool) { if attrs[0].a == a { return &attrs[0].s } + return nil } if len(attrs) == 2 { if attrs[0].a == a { @@ -130,11 +131,12 @@ func (s *span) AttributeFor(a traceql.Attribute) (traceql.Static, bool) { if attrs[1].a == a { return &attrs[1].s } + return nil } - for _, st := range attrs { - if st.a == a { - return &st.s + for i := range attrs { + if attrs[i].a == a { + return &attrs[i].s } } return nil @@ -144,6 +146,7 @@ func (s *span) AttributeFor(a traceql.Attribute) (traceql.Static, bool) { if attrs[0].a.Name == s { return &attrs[0].s } + return nil } if len(attrs) == 2 { if attrs[0].a.Name == s { @@ -152,47 +155,53 @@ func (s *span) AttributeFor(a traceql.Attribute) (traceql.Static, bool) { if attrs[1].a.Name == s { return &attrs[1].s } + return nil } - for _, st := range attrs { - if st.a.Name == s { - return &st.s + for i := range attrs { + if attrs[i].a.Name == s { + return &attrs[i].s } } return nil } - if a.Scope == traceql.AttributeScopeResource { - if attr := find(a, s.resourceAttrs); attr != nil { - return *attr, true + switch a.Scope { + case traceql.AttributeScopeResource: + if len(s.resourceAttrs) > 0 { + if attr := find(a, s.resourceAttrs); attr != nil { + return *attr, true + } } - return traceql.NewStaticNil(), false - } - - if a.Scope == traceql.AttributeScopeSpan { - if attr := find(a, s.spanAttrs); attr != nil { - return *attr, true + return traceql.StaticNil, false + case traceql.AttributeScopeSpan: + if len(s.spanAttrs) > 0 { + if attr := find(a, s.spanAttrs); attr != nil { + return *attr, true + } } - return traceql.NewStaticNil(), false - } - - if a.Scope == traceql.AttributeScopeEvent { - if attr := find(a, s.eventAttrs); attr != nil { - return *attr, true + return traceql.StaticNil, false + case traceql.AttributeScopeEvent: + if len(s.eventAttrs) > 0 { + if attr := find(a, s.eventAttrs); attr != nil { + return *attr, true + } } - return traceql.NewStaticNil(), false - } - if a.Scope == traceql.AttributeScopeLink { - if attr := find(a, s.linkAttrs); attr != nil { - return *attr, true + return traceql.StaticNil, false + case traceql.AttributeScopeLink: + if len(s.linkAttrs) > 0 { + if attr := find(a, s.linkAttrs); attr != nil { + return *attr, true + } } - return traceql.NewStaticNil(), false - } - if a.Scope == traceql.AttributeScopeInstrumentation { - if attr := find(a, s.instrumentationAttrs); attr != nil { - return *attr, true + return traceql.StaticNil, false + case traceql.AttributeScopeInstrumentation: + if len(s.instrumentationAttrs) > 0 { + if attr := find(a, s.instrumentationAttrs); attr != nil { + return *attr, true + } } - return traceql.NewStaticNil(), false + return traceql.StaticNil, false } if a.Intrinsic != traceql.IntrinsicNone { @@ -230,27 +239,37 @@ func (s *span) AttributeFor(a traceql.Attribute) (traceql.Static, bool) { // name search in span, resource, link, and event to give precedence to span // we don't need to do a name search at the trace level b/c it is intrinsics only - if attr := findName(a.Name, s.spanAttrs); attr != nil { - return *attr, true + if len(s.spanAttrs) > 0 { + if attr := findName(a.Name, s.spanAttrs); attr != nil { + return *attr, true + } } - if attr := findName(a.Name, s.resourceAttrs); attr != nil { - return *attr, true + if len(s.resourceAttrs) > 0 { + if attr := findName(a.Name, s.resourceAttrs); attr != nil { + return *attr, true + } } - if attr := findName(a.Name, s.eventAttrs); attr != nil { - return *attr, true + if len(s.eventAttrs) > 0 { + if attr := findName(a.Name, s.eventAttrs); attr != nil { + return *attr, true + } } - if attr := findName(a.Name, s.linkAttrs); attr != nil { - return *attr, true + if len(s.linkAttrs) > 0 { + if attr := findName(a.Name, s.linkAttrs); attr != nil { + return *attr, true + } } - if attr := findName(a.Name, s.instrumentationAttrs); attr != nil { - return *attr, true + if len(s.instrumentationAttrs) > 0 { + if attr := findName(a.Name, s.instrumentationAttrs); attr != nil { + return *attr, true + } } - return traceql.NewStaticNil(), false + return traceql.StaticNil, false } func (s *span) ID() []byte {