Seems like a reasonable proposal but I'd rather not go/types become dependent on context.Context.

How about just a function func() (installed in types.Config) that is called intermittently from go/types and which can be used to do whatever you want?

One could even make this function have simple API (signature) that could be used to report progress/and or tracing, if one were so inclined.

If there are useful pieces of metadata or actions beyond cancelation that are worth exposing then your suggestion sounds great to me.

Something like

type Config {
  ...
  Checkpoint func(*CheckerStatus)
}

type CheckerStatus struct {}
func (*CheckerStatus) Cancel() {}
...

If we want the actual context.Canceled error itself could get propagated, we could change Cancel() to take an error, or make the Checkpoint func return an error in lieu of Cancel() altogether.

I defer to you on whether there is other useful stuff that could take advantage of the generic callback. If there isn't other stuff in particular, then another option is adding (*go/types.Checker).Cancel().

I'm proposing a simpler solution, more akin to:

diff --git a/src/go/types/api.go b/src/go/types/api.go
index 2a21ad0c53..7ce2274ad2 100644
--- a/src/go/types/api.go
+++ b/src/go/types/api.go
@@ -114,6 +114,13 @@ type Config struct {
        // error found.
        Error func(err error)
 
+       // If notify is != nil, it is called repeatedly during type-checking
+       // with a notification message indicating progress. The frequency or
+       // message is not further specified (the message may just be a ".").
+       // Another use for Notify is to abort type-checking early by raising
+       // a panic.
+       Notify func(fmt.Stringer)
+
        // An importer is used to import packages referred to from
        // import declarations.
        // If the installed importer implements ImporterFrom, the type
diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index a4fb2b81cc..10e2f0b4bd 100644
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -61,6 +61,10 @@ func (check *Checker) objDecl(obj Object, def *Named) {
                }()
        }
 
+       if check.conf.Notify != nil {
+               check.conf.Notify(obj)
+       }
+
        // Checking the declaration of obj means inferring its type
        // (and possibly its value, for constants).
        // An object's type (and thus the object) may be in one of

The installed Notify will cause a specific panic (designed by the client) if type-checking should be aborted. A defer enveloping the invocation of the type-checker will catch that specific panic and with that, type-checking is aborted.

cc: @alandonovan for input

Is this necessary?

The unit of type checking (and thus cancellation of type checking, today) is the package, and all but the largest packages type-check very quickly, in less than a typical keypress interval when editing. I don't know the implementation of gopls, but does it type check the complete source each time, or does it destroy function bodies for functions in files other than those of interest? If it did that optimization, and implemented cancelation at package granularity (and perhaps de-bounced the input event stream?) I imagine it would be fast enough.

@alandonovan Good question; I don't know. @muirrn ?

Also, something that comes to mind: It may the import processing where the time goes since importing one package may require type-checking that package's imports and the package itself (though all those things should be cached). But definitively worthwhile looking into time spent in an importer.

gopls does do the optimization with function bodies, and all dependencies are cached. I think that this issue was raised in response to a specific case where a user holds down a key for an extended period of time, causing the behavior @muirrn described.

There are definitely plenty of optimizations that can still be made within gopls, but type-checking does account for a significant fraction of our memory allocations, so it would be helpful if it were cancelable. In the example of a user holding down a key--if type-checking begins before the next key-press is registered, we are unable to cancel it and end up doing unneeded work.

perhaps de-bounced the input event stream?

The main problem is synchronous LSP commands like completion. Editors often issue a completion request after every keypress, and to minimize completion latency gopls begins to type check ASAP. Any debouncing would reduce responsiveness.

Understood, but completion needs to type-check at most one function body, plus all the package-level declarations of a package. It should run in well under a millisecond.

In addition to completion and other language features, gopls provides diagnostics (which can include type-checking and analysis) on every keystroke, and this requires full function bodies for every file in a given package (and occasionally reverse dependencies).

With the current gopls caching, we actually explicitly don't cancel type-checking so that we can save the results for later use (see memoize.go). We would definitely have to rethink this if we could cancel type-checking. I still think it would be a net benefit, particularly since we would still be caching dependencies even if we canceled early.

Our concern is mostly CPU and memory usage, rather than speed. I'm currently debugging a memory leak 🙃, so I don't have accurate metrics yet, but once that is fixed, I will provide some more concrete data as justification.

By its nature, the results of completion are required with very low latency as they help the user choose what to type, but diagnostics don't make sense until the program is at least reasonably well formed. Issuing diagnostic requests on every keystroke seems likely to lead to cascades of errors, distracting UI updates, wasted CPU cycles, and drained batteries. I don't think it's a strong justification for this feature.

After some offline discussion with @alandonovan, I think our best approach here would be to check the context inside of the calls to (types.Importer).Import. That maybe enough for now, and it would ensure that we don't too much wasted work.

BTW, although the old go/loader historically did actual work in the import function, go/packages, for many good reasons, does the work of loading and type checking dependencies of package P before even beginning to type-check P, so the Import function is just a map lookup. But logically, yes, we could profitably break off a cancelled type-checking task after loading only some of the dependencies.

Thanks everyone. Closing as "rejected".

BTW I really like the idea of doing a minor type check with only the containing function and package decls for synchronous commands like completion. That would make completions snappy even on giant packages (assuming the package isn't one giant function...).

Change https://golang.org/cl/217257 mentions this issue: internal/lsp/cache: add context cancellation check inside importerFunc