Skip to content

Commit 504de27

Browse files
committed
internal/lsp: connect memoize actions to their callers
This adds the ability to tie a background context to the context that created it in traces, and also cleans up and annotates the context used in type checking. This gives us detailed connected traces of all the type checking and parsing logic. Change-Id: I32721220a50ecb9b4404a4e9354343389d7a5219 Reviewed-on: https://go-review.googlesource.com/c/tools/+/183757 Reviewed-by: Rebecca Stambler <[email protected]>
1 parent 4457e4c commit 504de27

File tree

7 files changed

+72
-20
lines changed

7 files changed

+72
-20
lines changed

internal/lsp/cache/check.go

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"golang.org/x/tools/go/analysis"
1717
"golang.org/x/tools/go/packages"
1818
"golang.org/x/tools/internal/lsp/source"
19+
"golang.org/x/tools/internal/lsp/telemetry/trace"
1920
"golang.org/x/tools/internal/span"
2021
)
2122

@@ -34,18 +35,19 @@ type importer struct {
3435
}
3536

3637
func (imp *importer) Import(pkgPath string) (*types.Package, error) {
38+
ctx := imp.ctx
3739
id, ok := imp.view.mcache.ids[packagePath(pkgPath)]
3840
if !ok {
3941
return nil, fmt.Errorf("no known ID for %s", pkgPath)
4042
}
41-
pkg, err := imp.getPkg(id)
43+
pkg, err := imp.getPkg(ctx, id)
4244
if err != nil {
4345
return nil, err
4446
}
4547
return pkg.types, nil
4648
}
4749

48-
func (imp *importer) getPkg(id packageID) (*pkg, error) {
50+
func (imp *importer) getPkg(ctx context.Context, id packageID) (*pkg, error) {
4951
if _, ok := imp.seen[id]; ok {
5052
return nil, fmt.Errorf("circular import detected")
5153
}
@@ -65,28 +67,30 @@ func (imp *importer) getPkg(id packageID) (*pkg, error) {
6567

6668
// This goroutine becomes responsible for populating
6769
// the entry and broadcasting its readiness.
68-
e.pkg, e.err = imp.typeCheck(id)
70+
e.pkg, e.err = imp.typeCheck(ctx, id)
6971
close(e.ready)
7072
}
7173

7274
if e.err != nil {
7375
// If the import had been previously canceled, and that error cached, try again.
74-
if e.err == context.Canceled && imp.ctx.Err() == nil {
76+
if e.err == context.Canceled && ctx.Err() == nil {
7577
imp.view.pcache.mu.Lock()
7678
// Clear out canceled cache entry if it is still there.
7779
if imp.view.pcache.packages[id] == e {
7880
delete(imp.view.pcache.packages, id)
7981
}
8082
imp.view.pcache.mu.Unlock()
81-
return imp.getPkg(id)
83+
return imp.getPkg(ctx, id)
8284
}
8385
return nil, e.err
8486
}
8587

8688
return e.pkg, nil
8789
}
8890

89-
func (imp *importer) typeCheck(id packageID) (*pkg, error) {
91+
func (imp *importer) typeCheck(ctx context.Context, id packageID) (*pkg, error) {
92+
ctx, ts := trace.StartSpan(ctx, "cache.importer.typeCheck")
93+
defer ts.End()
9094
meta, ok := imp.view.mcache.packages[id]
9195
if !ok {
9296
return nil, fmt.Errorf("no metadata for %v", id)
@@ -119,11 +123,11 @@ func (imp *importer) typeCheck(id packageID) (*pkg, error) {
119123
)
120124
for _, filename := range meta.files {
121125
uri := span.FileURI(filename)
122-
f, err := imp.view.getFile(imp.ctx, uri)
126+
f, err := imp.view.getFile(ctx, uri)
123127
if err != nil {
124128
continue
125129
}
126-
ph := imp.view.session.cache.ParseGoHandle(f.Handle(imp.ctx), mode)
130+
ph := imp.view.session.cache.ParseGoHandle(f.Handle(ctx), mode)
127131
phs = append(phs, ph)
128132
files = append(files, &astFile{
129133
uri: ph.File().Identity().URI,
@@ -136,7 +140,7 @@ func (imp *importer) typeCheck(id packageID) (*pkg, error) {
136140
go func(i int, ph source.ParseGoHandle) {
137141
defer wg.Done()
138142

139-
files[i].file, files[i].err = ph.Parse(imp.ctx)
143+
files[i].file, files[i].err = ph.Parse(ctx)
140144
}(i, ph)
141145
}
142146
wg.Wait()
@@ -175,7 +179,7 @@ func (imp *importer) typeCheck(id packageID) (*pkg, error) {
175179
IgnoreFuncBodies: mode == source.ParseExported,
176180
Importer: &importer{
177181
view: imp.view,
178-
ctx: imp.ctx,
182+
ctx: ctx,
179183
fset: imp.fset,
180184
topLevelPkgID: imp.topLevelPkgID,
181185
seen: seen,
@@ -187,7 +191,7 @@ func (imp *importer) typeCheck(id packageID) (*pkg, error) {
187191
check.Files(pkg.GetSyntax())
188192

189193
// Add every file in this package to our cache.
190-
if err := imp.cachePackage(imp.ctx, pkg, meta, mode); err != nil {
194+
if err := imp.cachePackage(ctx, pkg, meta, mode); err != nil {
191195
return nil, err
192196
}
193197

@@ -213,7 +217,7 @@ func (imp *importer) cachePackage(ctx context.Context, pkg *pkg, meta *metadata,
213217
// We lock the package cache, but we shouldn't get any inconsistencies
214218
// because we are still holding the lock on the view.
215219
for importPath := range meta.children {
216-
importPkg, err := imp.getPkg(importPath)
220+
importPkg, err := imp.getPkg(ctx, importPath)
217221
if err != nil {
218222
continue
219223
}

internal/lsp/cache/load.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,10 @@ func (v *view) loadParseTypecheck(ctx context.Context, f *goFile) ([]packages.Er
4343
}
4444
// Start prefetching direct imports.
4545
for importID := range m.children {
46-
go imp.getPkg(importID)
46+
go imp.getPkg(ctx, importID)
4747
}
4848
// Type-check package.
49-
pkg, err := imp.getPkg(imp.topLevelPkgID)
49+
pkg, err := imp.getPkg(ctx, imp.topLevelPkgID)
5050
if err != nil {
5151
return nil, err
5252
}

internal/lsp/protocol/detatch.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package protocol
6+
7+
import (
8+
"context"
9+
"time"
10+
)
11+
12+
// detatch returns a context that keeps all the values of its parent context
13+
// but detatches from the cancellation and error handling.
14+
func detatchContext(ctx context.Context) context.Context { return detatchedContext{ctx} }
15+
16+
type detatchedContext struct{ parent context.Context }
17+
18+
func (v detatchedContext) Deadline() (time.Time, bool) { return time.Time{}, false }
19+
func (v detatchedContext) Done() <-chan struct{} { return nil }
20+
func (v detatchedContext) Err() error { return nil }
21+
func (v detatchedContext) Value(key interface{}) interface{} { return v.parent.Value(key) }

internal/lsp/protocol/protocol.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,18 @@ import (
88
"context"
99

1010
"golang.org/x/tools/internal/jsonrpc2"
11+
"golang.org/x/tools/internal/lsp/telemetry/trace"
1112
"golang.org/x/tools/internal/lsp/xlog"
1213
)
1314

1415
const defaultMessageBufferSize = 20
1516
const defaultRejectIfOverloaded = false
1617

1718
func canceller(ctx context.Context, conn *jsonrpc2.Conn, id jsonrpc2.ID) {
18-
conn.Notify(context.Background(), "$/cancelRequest", &CancelParams{ID: id})
19+
ctx = detatchContext(ctx)
20+
ctx, span := trace.StartSpan(ctx, "protocol.canceller")
21+
defer span.End()
22+
conn.Notify(ctx, "$/cancelRequest", &CancelParams{ID: id})
1923
}
2024

2125
func NewClient(stream jsonrpc2.Stream, client Client) (*jsonrpc2.Conn, Server, xlog.Logger) {

internal/lsp/telemetry/trace/trace.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,12 @@
55
// Package tag adds support for telemetry tracins.
66
package trace
77

8-
import "context"
8+
import (
9+
"context"
10+
)
911

1012
type Span interface {
1113
AddAttributes(attributes ...Attribute)
12-
1314
AddMessageReceiveEvent(messageID, uncompressedByteSize, compressedByteSize int64)
1415
AddMessageSendEvent(messageID, uncompressedByteSize, compressedByteSize int64)
1516
Annotate(attributes []Attribute, str string)
@@ -41,6 +42,7 @@ func (nullSpan) SetStatus(status Status)
4142

4243
var (
4344
FromContext = func(ctx context.Context) Span { return nullSpan{} }
45+
NewContext = func(ctx context.Context, span Span) context.Context { return ctx }
4446
StartSpan = func(ctx context.Context, name string, options ...interface{}) (context.Context, Span) {
4547
return ctx, nullSpan{}
4648
}

internal/memoize/detatch.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright 2019 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package memoize
6+
7+
import (
8+
"context"
9+
"time"
10+
)
11+
12+
// detatch returns a context that keeps all the values of its parent context
13+
// but detatches from the cancellation and error handling.
14+
func detatchContext(ctx context.Context) context.Context { return detatchedContext{ctx} }
15+
16+
type detatchedContext struct{ parent context.Context }
17+
18+
func (v detatchedContext) Deadline() (time.Time, bool) { return time.Time{}, false }
19+
func (v detatchedContext) Done() <-chan struct{} { return nil }
20+
func (v detatchedContext) Err() error { return nil }
21+
func (v detatchedContext) Value(key interface{}) interface{} { return v.parent.Value(key) }

internal/memoize/memoize.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ type Handle struct {
4646
// there is only one instance of the result live at any given time.
4747
type entry struct {
4848
noCopy
49+
key interface{}
4950
// mu contols access to the typ and ptr fields
5051
mu sync.Mutex
5152
// the calculated value, as stored in an interface{}
@@ -93,7 +94,7 @@ func (s *Store) Bind(key interface{}, function Function) *Handle {
9394
if s.entries == nil {
9495
s.entries = make(map[interface{}]*entry)
9596
}
96-
e = &entry{}
97+
e = &entry{key: key}
9798
s.entries[key] = e
9899
}
99100
return &Handle{
@@ -179,8 +180,7 @@ func (e *entry) get(ctx context.Context, f Function) (interface{}, bool) {
179180
// Use the background context to avoid canceling the function.
180181
// The function cannot be canceled even if the context is canceled
181182
// because multiple goroutines may depend on it.
182-
ctx := context.Background()
183-
value = f(ctx)
183+
value = f(detatchContext(ctx))
184184

185185
// The function has completed. Update the value in the entry.
186186
e.mu.Lock()

0 commit comments

Comments
 (0)