Skip to content

Commit 774d2ec

Browse files
ianthehatianthehat
committed
internal/lsp: cancel early
This change allows us to hanel cancel messages as they go into the queue, and cancel messages that are ahead of them in the queue but not being processed yet. This should reduce the amount of redundant work that we do when we are handling a cancel storm. Change-Id: Id1a58991407d75b68d65bacf96350a4dd69d4d2b Reviewed-on: https://go-review.googlesource.com/c/tools/+/200766 Run-TryBot: Ian Cottrell <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]>
1 parent 7178990 commit 774d2ec

File tree

8 files changed

+90
-78
lines changed

8 files changed

+90
-78
lines changed

‎internal/jsonrpc2/handler.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ type Handler interface {
3838
// response
3939

4040
// Request is called near the start of processing any request.
41-
Request(ctx context.Context, direction Direction, r *WireRequest) context.Context
41+
Request(ctx context.Context, conn *Conn, direction Direction, r *WireRequest) context.Context
4242
// Response is called near the start of processing any response.
43-
Response(ctx context.Context, direction Direction, r *WireResponse) context.Context
43+
Response(ctx context.Context, conn *Conn, direction Direction, r *WireResponse) context.Context
4444
// Done is called when any request is fully processed.
4545
// For calls, this means the response has also been processed, for notifies
4646
// this is as soon as the message has been written to the stream.
@@ -90,11 +90,11 @@ func (EmptyHandler) Cancel(ctx context.Context, conn *Conn, id ID, cancelled boo
9090
return false
9191
}
9292

93-
func (EmptyHandler) Request(ctx context.Context, direction Direction, r *WireRequest) context.Context {
93+
func (EmptyHandler) Request(ctx context.Context, conn *Conn, direction Direction, r *WireRequest) context.Context {
9494
return ctx
9595
}
9696

97-
func (EmptyHandler) Response(ctx context.Context, direction Direction, r *WireResponse) context.Context {
97+
func (EmptyHandler) Response(ctx context.Context, conn *Conn, direction Direction, r *WireResponse) context.Context {
9898
return ctx
9999
}
100100

‎internal/jsonrpc2/jsonrpc2.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func (c *Conn) Notify(ctx context.Context, method string, params interface{}) (e
110110
return fmt.Errorf("marshalling notify request: %v", err)
111111
}
112112
for _, h := range c.handlers {
113-
ctx = h.Request(ctx, Send, request)
113+
ctx = h.Request(ctx, c, Send, request)
114114
}
115115
defer func() {
116116
for _, h := range c.handlers {
@@ -145,7 +145,7 @@ func (c *Conn) Call(ctx context.Context, method string, params, result interface
145145
return fmt.Errorf("marshalling call request: %v", err)
146146
}
147147
for _, h := range c.handlers {
148-
ctx = h.Request(ctx, Send, request)
148+
ctx = h.Request(ctx, c, Send, request)
149149
}
150150
// we have to add ourselves to the pending map before we send, otherwise we
151151
// are racing the response
@@ -175,7 +175,7 @@ func (c *Conn) Call(ctx context.Context, method string, params, result interface
175175
select {
176176
case response := <-rchan:
177177
for _, h := range c.handlers {
178-
ctx = h.Response(ctx, Receive, response)
178+
ctx = h.Response(ctx, c, Receive, response)
179179
}
180180
// is it an error response?
181181
if response.Error != nil {
@@ -262,7 +262,7 @@ func (r *Request) Reply(ctx context.Context, result interface{}, err error) erro
262262
return err
263263
}
264264
for _, h := range r.conn.handlers {
265-
ctx = h.Response(ctx, Send, response)
265+
ctx = h.Response(ctx, r.conn, Send, response)
266266
}
267267
n, err := r.conn.stream.Write(ctx, data)
268268
for _, h := range r.conn.handlers {
@@ -347,7 +347,7 @@ func (c *Conn) Run(runCtx context.Context) error {
347347
},
348348
}
349349
for _, h := range c.handlers {
350-
reqCtx = h.Request(reqCtx, Receive, &req.WireRequest)
350+
reqCtx = h.Request(reqCtx, c, Receive, &req.WireRequest)
351351
reqCtx = h.Read(reqCtx, n)
352352
}
353353
c.setHandling(req, true)

‎internal/jsonrpc2/jsonrpc2_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func (h *handle) Cancel(ctx context.Context, conn *jsonrpc2.Conn, id jsonrpc2.ID
164164
return false
165165
}
166166

167-
func (h *handle) Request(ctx context.Context, direction jsonrpc2.Direction, r *jsonrpc2.WireRequest) context.Context {
167+
func (h *handle) Request(ctx context.Context, conn *jsonrpc2.Conn, direction jsonrpc2.Direction, r *jsonrpc2.WireRequest) context.Context {
168168
if h.log {
169169
if r.ID != nil {
170170
log.Printf("%v call [%v] %s %v", direction, r.ID, r.Method, r.Params)
@@ -177,7 +177,7 @@ func (h *handle) Request(ctx context.Context, direction jsonrpc2.Direction, r *j
177177
return ctx
178178
}
179179

180-
func (h *handle) Response(ctx context.Context, direction jsonrpc2.Direction, r *jsonrpc2.WireResponse) context.Context {
180+
func (h *handle) Response(ctx context.Context, conn *jsonrpc2.Conn, direction jsonrpc2.Direction, r *jsonrpc2.WireResponse) context.Context {
181181
if h.log {
182182
method := ctx.Value("method")
183183
elapsed := time.Since(ctx.Value("start").(time.Time))

‎internal/lsp/cmd/serve.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (h *handler) Cancel(ctx context.Context, conn *jsonrpc2.Conn, id jsonrpc2.I
149149
return false
150150
}
151151

152-
func (h *handler) Request(ctx context.Context, direction jsonrpc2.Direction, r *jsonrpc2.WireRequest) context.Context {
152+
func (h *handler) Request(ctx context.Context, conn *jsonrpc2.Conn, direction jsonrpc2.Direction, r *jsonrpc2.WireRequest) context.Context {
153153
if r.Method == "" {
154154
panic("no method in rpc stats")
155155
}
@@ -174,7 +174,7 @@ func (h *handler) Request(ctx context.Context, direction jsonrpc2.Direction, r *
174174
return ctx
175175
}
176176

177-
func (h *handler) Response(ctx context.Context, direction jsonrpc2.Direction, r *jsonrpc2.WireResponse) context.Context {
177+
func (h *handler) Response(ctx context.Context, conn *jsonrpc2.Conn, direction jsonrpc2.Direction, r *jsonrpc2.WireResponse) context.Context {
178178
return ctx
179179
}
180180

‎internal/lsp/protocol/protocol.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,19 @@ package protocol
66

77
import (
88
"context"
9+
"encoding/json"
910

1011
"golang.org/x/tools/internal/jsonrpc2"
1112
"golang.org/x/tools/internal/telemetry/log"
1213
"golang.org/x/tools/internal/telemetry/trace"
1314
"golang.org/x/tools/internal/xcontext"
1415
)
1516

17+
const (
18+
// RequestCancelledError should be used when a request is cancelled early.
19+
RequestCancelledError = -32800
20+
)
21+
1622
type DocumentUri = string
1723

1824
type canceller struct{ jsonrpc2.EmptyHandler }
@@ -27,6 +33,18 @@ type serverHandler struct {
2733
server Server
2834
}
2935

36+
func (canceller) Request(ctx context.Context, conn *jsonrpc2.Conn, direction jsonrpc2.Direction, r *jsonrpc2.WireRequest) context.Context {
37+
if direction == jsonrpc2.Receive && r.Method == "$/cancelRequest" {
38+
var params CancelParams
39+
if err := json.Unmarshal(*r.Params, &params); err != nil {
40+
log.Error(ctx, "", err)
41+
} else {
42+
conn.Cancel(params.ID)
43+
}
44+
}
45+
return ctx
46+
}
47+
3048
func (canceller) Cancel(ctx context.Context, conn *jsonrpc2.Conn, id jsonrpc2.ID, cancelled bool) bool {
3149
if cancelled {
3250
return false

‎internal/lsp/protocol/tsclient.go

Lines changed: 6 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/lsp/protocol/tsserver.go

Lines changed: 45 additions & 47 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/lsp/protocol/typescript/requests.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,8 @@ function output(side: side) {
224224
225225
"golang.org/x/tools/internal/jsonrpc2"
226226
"golang.org/x/tools/internal/telemetry/log"
227-
)
227+
"golang.org/x/tools/internal/xcontext"
228+
)
228229
`);
229230
const a = side.name[0].toUpperCase() + side.name.substring(1)
230231
f(`type ${a} interface {`);
@@ -235,15 +236,12 @@ function output(side: side) {
235236
if delivered {
236237
return false
237238
}
238-
switch r.Method {
239-
case "$/cancelRequest":
240-
var params CancelParams
241-
if err := json.Unmarshal(*r.Params, &params); err != nil {
242-
sendParseError(ctx, r, err)
243-
return true
244-
}
245-
r.Conn().Cancel(params.ID)
246-
return true`);
239+
if ctx.Err() != nil {
240+
ctx := xcontext.Detach(ctx)
241+
r.Reply(ctx, nil, jsonrpc2.NewErrorf(RequestCancelledError, ""))
242+
return true
243+
}
244+
switch r.Method {`);
247245
side.cases.forEach((v) => {f(v)});
248246
f(`
249247
default:

0 commit comments

Comments
 (0)