internal/lsp: make tag iteration allocation-free

Change the tag iteration api to something less flexible
that allows for event iteration without allocation.

Change-Id: I212d45ebceea0183d1a61e6b611e0558649be60a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/227301
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>

internal/lsp/debug/trace.go

@@ -94,7 +94,7 @@ func (t *traces) ProcessEvent(ctx context.Context, ev event.Event, tags event.Ta
 			ParentID: span.ParentID,
 			Name:     span.Name,
 			Start:    span.Start().At,
-			Tags:     renderTags(span.Start().Tags()),
+			Tags:     renderTags(span.Start()),
 		}
 		t.unfinished[span.ID] = td
 		// and wire up parents if we have them
@@ -124,7 +124,7 @@ func (t *traces) ProcessEvent(ctx context.Context, ev event.Event, tags event.Ta
 		for i, event := range events {
 			td.Events[i] = traceEvent{
 				Time: event.At,
-				Tags: renderTags(event.Tags()),
+				Tags: renderTags(event),
 			}
 		}
 
@@ -170,10 +170,12 @@ func fillOffsets(td *traceData, start time.Time) {
 	}
 }
 
-func renderTags(tags event.TagIterator) string {
+func renderTags(tags event.TagList) string {
 	buf := &bytes.Buffer{}
-	for ; tags.Valid(); tags.Advance() {
-		fmt.Fprintf(buf, "%v ", tags.Tag())
+	for index := 0; tags.Valid(index); index++ {
+		if tag := tags.Tag(index); tag.Valid() {
+			fmt.Fprintf(buf, "%v ", tag)
+		}
 	}
 	return buf.String()
 }

internal/telemetry/event/event.go

@@ -68,21 +68,26 @@ func (ev Event) Format(f fmt.State, r rune) {
 			fmt.Fprintf(f, ": %v", err)
 		}
 	}
-	for it := ev.Tags(); it.Valid(); it.Advance() {
-		tag := it.Tag()
+	for index := 0; ev.Valid(index); index++ {
+		tag := ev.Tag(index)
 		// msg and err were both already printed above, so we skip them to avoid
 		// double printing
-		if tag.Key == Msg || tag.Key == Err {
+		if !tag.Valid() || tag.Key == Msg || tag.Key == Err {
 			continue
 		}
 		fmt.Fprintf(f, "\n\t%v", tag)
 	}
 }
 
-func (ev Event) Tags() TagIterator {
-	return ChainTagIterators(
-		NewTagIterator(ev.static[:]...),
-		NewTagIterator(ev.dynamic...))
+func (ev Event) Valid(index int) bool {
+	return index >= 0 && index < len(ev.static)+len(ev.dynamic)
+}
+
+func (ev Event) Tag(index int) Tag {
+	if index < len(ev.static) {
+		return ev.static[index]
+	}
+	return ev.dynamic[index-len(ev.static)]
 }
 
 func (ev Event) Find(key interface{}) Tag {

internal/telemetry/event/tag.go

@@ -23,38 +23,26 @@ type TagMap interface {
 	Find(key interface{}) Tag
 }
 
-// TagPointer is the interface to something that provides an iterable
+// TagList is the interface to something that provides an iterable
 // list of tags.
-type TagPointer interface {
-	// Next advances to the next entry in the list and return a TagIterator for it.
-	// It will return nil if there are no more entries.
-	Next() TagPointer
-	// Tag returns the tag the pointer is for.
-	Tag() Tag
+// Iteration should start from 0 and continue until Valid returns false.
+type TagList interface {
+	// Valid returns true if the index is within range for the list.
+	// It does not imply the tag at that index will itself be valid.
+	Valid(index int) bool
+	// Tag returns the tag at the given index.
+	Tag(index int) Tag
 }
 
-// TagIterator is used to iterate through tags using TagPointer.
-// It is a small helper that will normally fully inline to make it easier to
-// manage the fact that pointer advance returns a new pointer rather than
-// moving the existing one.
-type TagIterator struct {
-	ptr TagPointer
-}
-
-// tagPointer implements TagPointer over a simple list of tags.
-type tagPointer struct {
+// tagList implements TagList for a list of Tags.
+type tagList struct {
 	tags []Tag
 }
 
-// tagPointer wraps a TagPointer filtering out specific tags.
+// tagFilter wraps a TagList filtering out specific tags.
 type tagFilter struct {
-	filter     []Key
-	underlying TagPointer
-}
-
-// tagPointerChain implements TagMap for a list of underlying TagMap.
-type tagPointerChain struct {
-	ptrs []TagPointer
+	keys       []Key
+	underlying TagList
 }
 
 // tagMap implements TagMap for a simple list of tags.
@@ -114,79 +102,26 @@ func (t Tag) Format(f fmt.State, r rune) {
 	}
 }
 
-func (i *TagIterator) Valid() bool {
-	return i.ptr != nil
+func (l *tagList) Valid(index int) bool {
+	return index >= 0 && index < len(l.tags)
 }
 
-func (i *TagIterator) Advance() {
-	i.ptr = i.ptr.Next()
+func (l *tagList) Tag(index int) Tag {
+	return l.tags[index]
 }
 
-func (i *TagIterator) Tag() Tag {
-	return i.ptr.Tag()
+func (f *tagFilter) Valid(index int) bool {
+	return f.underlying.Valid(index)
 }
 
-func (i tagPointer) Next() TagPointer {
-	// loop until we are on a valid tag
-	for {
-		// move on one tag
-		i.tags = i.tags[1:]
-		// check if we have exhausted the current list
-		if len(i.tags) == 0 {
-			// no more tags, so no more iterator
-			return nil
-		}
-		// if the tag is valid, we are done
-		if i.tags[0].Valid() {
-			return i
-		}
-	}
-}
-
-func (i tagPointer) Tag() Tag {
-	return i.tags[0]
-}
-
-func (i tagFilter) Next() TagPointer {
-	// loop until we are on a valid tag
-	for {
-		i.underlying = i.underlying.Next()
-		if i.underlying == nil {
-			return nil
-		}
-		if !i.filtered() {
-			return i
-		}
-	}
-}
-
-func (i tagFilter) filtered() bool {
-	tag := i.underlying.Tag()
-	for _, f := range i.filter {
+func (f *tagFilter) Tag(index int) Tag {
+	tag := f.underlying.Tag(index)
+	for _, f := range f.keys {
 		if tag.Key == f {
-			return true
+			return Tag{}
 		}
 	}
-	return false
-}
-
-func (i tagFilter) Tag() Tag {
-	return i.underlying.Tag()
-}
-
-func (i tagPointerChain) Next() TagPointer {
-	i.ptrs[0] = i.ptrs[0].Next()
-	if i.ptrs[0] == nil {
-		i.ptrs = i.ptrs[1:]
-	}
-	if len(i.ptrs) == 0 {
-		return nil
-	}
-	return i
-}
-
-func (i tagPointerChain) Tag() Tag {
-	return i.ptrs[0].Tag()
+	return tag
 }
 
 func (l tagMap) Find(key interface{}) Tag {
@@ -208,43 +143,20 @@ func (c tagMapChain) Find(key interface{}) Tag {
 	return Tag{}
 }
 
-func NewTagIterator(tags ...Tag) TagIterator {
+var emptyList = &tagList{}
+
+func NewTagList(tags ...Tag) TagList {
 	if len(tags) == 0 {
-		return TagIterator{}
+		return emptyList
 	}
-	result := TagIterator{ptr: tagPointer{tags: tags}}
-	if !result.Tag().Valid() {
-		result.Advance()
-	}
-	return result
+	return &tagList{tags: tags}
 }
 
-func Filter(it TagIterator, keys ...Key) TagIterator {
-	if !it.Valid() || len(keys) == 0 {
-		return it
+func Filter(l TagList, keys ...Key) TagList {
+	if len(keys) == 0 {
+		return l
 	}
-	ptr := tagFilter{filter: keys, underlying: it.ptr}
-	result := TagIterator{ptr: ptr}
-	if ptr.filtered() {
-		result.Advance()
-	}
-	return result
-}
-
-func ChainTagIterators(iterators ...TagIterator) TagIterator {
-	if len(iterators) == 0 {
-		return TagIterator{}
-	}
-	ptrs := make([]TagPointer, 0, len(iterators))
-	for _, it := range iterators {
-		if it.Valid() {
-			ptrs = append(ptrs, it.ptr)
-		}
-	}
-	if len(ptrs) == 0 {
-		return TagIterator{}
-	}
-	return TagIterator{ptr: tagPointerChain{ptrs: ptrs}}
+	return &tagFilter{keys: keys, underlying: l}
 }
 
 func NewTagMap(tags ...Tag) TagMap {

internal/telemetry/event/tag_test.go

@@ -22,7 +22,7 @@ var (
 	all  = []event.Tag{A, B, C}
 )
 
-func TestTagIterator(t *testing.T) {
+func TestTagList(t *testing.T) {
 	for _, test := range []struct {
 		name   string
 		tags   []event.Tag
@@ -71,7 +71,7 @@ func TestTagIterator(t *testing.T) {
 		expect: `A="a"`,
 	}} {
 		t.Run(test.name, func(t *testing.T) {
-			got := printIterator(event.NewTagIterator(test.tags...))
+			got := printList(event.NewTagList(test.tags...))
 			if got != test.expect {
 				t.Errorf("got %q want %q", got, test.expect)
 			}
@@ -115,54 +115,8 @@ func TestTagFilter(t *testing.T) {
 		expect:  `B="b"`,
 	}} {
 		t.Run(test.name, func(t *testing.T) {
-			tags := event.NewTagIterator(test.tags...)
-			got := printIterator(event.Filter(tags, test.filters...))
-			if got != test.expect {
-				t.Errorf("got %q want %q", got, test.expect)
-			}
-		})
-	}
-}
-
-func TestTagChain(t *testing.T) {
-	for _, test := range []struct {
-		name   string
-		tags   [][]event.Tag
-		expect string
-	}{{
-		name:   "no iterators",
-		expect: ``,
-	}, {
-		name:   "one iterator",
-		tags:   [][]event.Tag{all},
-		expect: `A="a", B="b", C="c"`,
-	}, {
-		name:   "invalid iterator",
-		tags:   [][]event.Tag{{}},
-		expect: ``,
-	}, {
-		name:   "two iterators",
-		tags:   [][]event.Tag{{B, C}, {A}},
-		expect: `B="b", C="c", A="a"`,
-	}, {
-		name:   "invalid start iterator",
-		tags:   [][]event.Tag{{}, {B, C}},
-		expect: `B="b", C="c"`,
-	}, {
-		name:   "invalid mid iterator",
-		tags:   [][]event.Tag{{A}, {}, {C}},
-		expect: `A="a", C="c"`,
-	}, {
-		name:   "invalid end iterator",
-		tags:   [][]event.Tag{{B, C}, {}},
-		expect: `B="b", C="c"`,
-	}} {
-		t.Run(test.name, func(t *testing.T) {
-			iterators := make([]event.TagIterator, len(test.tags))
-			for i, v := range test.tags {
-				iterators[i] = event.NewTagIterator(v...)
-			}
-			got := printIterator(event.ChainTagIterators(iterators...))
+			tags := event.NewTagList(test.tags...)
+			got := printList(event.Filter(tags, test.filters...))
 			if got != test.expect {
 				t.Errorf("got %q want %q", got, test.expect)
 			}
@@ -281,13 +235,17 @@ func TestTagMapMerge(t *testing.T) {
 	}
 }
 
-func printIterator(it event.TagIterator) string {
+func printList(l event.TagList) string {
 	buf := &bytes.Buffer{}
-	for ; it.Valid(); it.Advance() {
+	for index := 0; l.Valid(index); index++ {
+		tag := l.Tag(index)
+		if !tag.Valid() {
+			continue
+		}
 		if buf.Len() > 0 {
 			buf.WriteString(", ")
 		}
-		fmt.Fprint(buf, it.Tag())
+		fmt.Fprint(buf, tag)
 	}
 	return buf.String()
 }

internal/telemetry/export/metric/exporter.go

@@ -37,8 +37,11 @@ func (e *Config) Exporter(output event.Exporter) event.Exporter {
 		mu.Lock()
 		defer mu.Unlock()
 		var metrics []Data
-		for it := ev.Tags(); it.Valid(); it.Advance() {
-			tag := it.Tag()
+		for index := 0; ev.Valid(index); index++ {
+			tag := ev.Tag(index)
+			if !tag.Valid() {
+				continue
+			}
 			id := tag.Key
 			if list := e.subscribers[id]; len(list) > 0 {
 				for _, s := range list {

internal/telemetry/export/ocagent/ocagent.go

@@ -200,7 +200,7 @@ func convertSpan(span *export.Span) *wire.Span {
 		Kind:                    wire.UnspecifiedSpanKind,
 		StartTime:               convertTimestamp(span.Start().At),
 		EndTime:                 convertTimestamp(span.Finish().At),
-		Attributes:              convertAttributes(event.Filter(span.Start().Tags(), event.Name)),
+		Attributes:              convertAttributes(event.Filter(span.Start(), event.Name)),
 		TimeEvents:              convertEvents(span.Events()),
 		SameProcessAsParentSpan: true,
 		//TODO: StackTrace?
@@ -227,16 +227,32 @@ func convertMetric(data metric.Data, start time.Time) *wire.Metric {
 	}
 }
 
-func convertAttributes(it event.TagIterator) *wire.Attributes {
-	if !it.Valid() {
+func skipToValidTag(l event.TagList) (int, event.Tag) {
+	// skip to the first valid tag
+	for index := 0; l.Valid(index); index++ {
+		if tag := l.Tag(index); tag.Valid() {
+			return index, tag
+		}
+	}
+	return -1, event.Tag{}
+}
+
+func convertAttributes(l event.TagList) *wire.Attributes {
+	index, tag := skipToValidTag(l)
+	if !tag.Valid() {
 		return nil
 	}
 	attributes := make(map[string]wire.Attribute)
-	for ; it.Valid(); it.Advance() {
-		tag := it.Tag()
-		attributes[tag.Key.Name()] = convertAttribute(tag)
+	for {
+		if tag.Valid() {
+			attributes[tag.Key.Name()] = convertAttribute(tag)
+		}
+		index++
+		if !l.Valid(index) {
+			return &wire.Attributes{AttributeMap: attributes}
+		}
+		tag = l.Tag(index)
 	}
-	return &wire.Attributes{AttributeMap: attributes}
 }
 
 func convertAttribute(tag event.Tag) wire.Attribute {
@@ -295,13 +311,12 @@ func convertEvent(ev event.Event) wire.TimeEvent {
 }
 
 func convertAnnotation(ev event.Event) *wire.Annotation {
-	tags := ev.Tags()
-	if !tags.Valid() {
+	if _, tag := skipToValidTag(ev); !tag.Valid() {
 		return nil
 	}
 	tagMap := event.TagMap(ev)
 	description := event.Msg.Get(tagMap)
-	tags = event.Filter(tags, event.Msg)
+	tags := event.Filter(ev, event.Msg)
 	if description == "" {
 		err := event.Err.Get(tagMap)
 		tags = event.Filter(tags, event.Err)