internal/lsp/debug: reduce critical sections in trace

This change reduces the sizes of the critical section
in traces.ProcessEvent, in particular moving allocations
ahead of Lock. This reduces the contention according
to the trace profiler.

See https://go-review.googlesource.com/c/go/+/411909 for
another reduction in contention. The largest remaining
contention is Handle.Get, which thousands of goroutines
wait for because we initiate typechecking top down.

(Second attempt at https://go-review.googlesource.com/c/tools/+/411910,
reverted in https://go-review.googlesource.com/c/tools/+/412694.
The changes to Generation.Bind have been dropped.)

Change-Id: Ia9050c97bd12d2d75055f8d1dfcda3ef1f5ad334
Reviewed-on: https://go-review.googlesource.com/c/tools/+/412820
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>

internal/lsp/cache/parse.go

@@ -278,7 +278,7 @@ func parseGo(ctx context.Context, fset *token.FileSet, fh source.FileHandle, mod
 
 	tok := fset.File(file.Pos())
 	if tok == nil {
-		// file.Pos is the location of the package declaration. If there was
+		// file.Pos is the location of the package declaration (issue #53202). If there was
 		// none, we can't find the token.File that ParseFile created, and we
 		// have no choice but to recreate it.
 		tok = fset.AddFile(fh.URI().Filename(), -1, len(src))

internal/lsp/debug/trace.go

@@ -119,8 +119,6 @@ func formatEvent(ctx context.Context, ev core.Event, lm label.Map) string {
 }
 
 func (t *traces) ProcessEvent(ctx context.Context, ev core.Event, lm label.Map) context.Context {
-	t.mu.Lock()
-	defer t.mu.Unlock()
 	span := export.GetSpan(ctx)
 	if span == nil {
 		return ctx
@@ -128,11 +126,8 @@ func (t *traces) ProcessEvent(ctx context.Context, ev core.Event, lm label.Map)
 
 	switch {
 	case event.IsStart(ev):
-		if t.sets == nil {
-			t.sets = make(map[string]*traceSet)
-			t.unfinished = make(map[export.SpanContext]*traceData)
-		}
-		// just starting, add it to the unfinished map
+		// Just starting: add it to the unfinished map.
+		// Allocate before the critical section.
 		td := &traceData{
 			TraceID:  span.ID.TraceID,
 			SpanID:   span.ID.SpanID,
@@ -141,6 +136,13 @@ func (t *traces) ProcessEvent(ctx context.Context, ev core.Event, lm label.Map)
 			Start:    span.Start().At(),
 			Tags:     renderLabels(span.Start()),
 		}
+
+		t.mu.Lock()
+		defer t.mu.Unlock()
+		if t.sets == nil {
+			t.sets = make(map[string]*traceSet)
+			t.unfinished = make(map[export.SpanContext]*traceData)
+		}
 		t.unfinished[span.ID] = td
 		// and wire up parents if we have them
 		if !span.ParentID.IsValid() {
@@ -155,7 +157,19 @@ func (t *traces) ProcessEvent(ctx context.Context, ev core.Event, lm label.Map)
 		parent.Children = append(parent.Children, td)
 
 	case event.IsEnd(ev):
-		// finishing, must be already in the map
+		// Finishing: must be already in the map.
+		// Allocate events before the critical section.
+		events := span.Events()
+		tdEvents := make([]traceEvent, len(events))
+		for i, event := range events {
+			tdEvents[i] = traceEvent{
+				Time: event.At(),
+				Tags: renderLabels(event),
+			}
+		}
+
+		t.mu.Lock()
+		defer t.mu.Unlock()
 		td, found := t.unfinished[span.ID]
 		if !found {
 			return ctx // if this happens we are in a bad place
@@ -164,14 +178,7 @@ func (t *traces) ProcessEvent(ctx context.Context, ev core.Event, lm label.Map)
 
 		td.Finish = span.Finish().At()
 		td.Duration = span.Finish().At().Sub(span.Start().At())
-		events := span.Events()
-		td.Events = make([]traceEvent, len(events))
-		for i, event := range events {
-			td.Events[i] = traceEvent{
-				Time: event.At(),
-				Tags: renderLabels(event),
-			}
-		}
+		td.Events = tdEvents
 
 		set, ok := t.sets[span.Name]
 		if !ok {