Language-server-protocol: Add support to report progress for a running request

[dbaeumer]

[felixfbecker]

Moving my suggestions here

I see two possibilities here

• Go with the pattern of HTTP 202 Accepted. You send a request, and get back a response instantly stating the request has been "accepted". You can then do more requests with a different method, like $/progess (similar to cancel), passing the message ID of the initial request as a param, and get progress reports back.
• Go with a pattern similar to HTTP 102 Processing (WebDAV). You send a request, and before the actual response comes in, the server can decide to continuously send a Progress header, and finally the actual Content-Length header and response body.

[felixfbecker]

Proposal

Add a new notification window/progress that is sent from the server to the client.
Params:

interface ProgressParams {

  /** a unique identifier to associate multiple progress notifications with the same progress */
  id: string | number;

  /** 
    * an optional request ID, indicating that this is the progress of a specific request from the client
    * if set, the client can use the information to present a progress indicator in the context of the action that triggered the request
    * if not set, the progress is treated as arbitrary progress for an arbitrary async operation (for example indexing) and the client can show it in a general location, like a status bar
    */
  requestId?: number | string;

  /** number that ranges from zero to total */
  status: number;

  /**
    * optional total number that status is relative to.
    * If set, the client can choose to display this next to the progress indicator.
    * If not set, assumed to be 1.
    */
  total?: number;

  /**
    * optional label for the progress.
    * If the client decides to use it, it should be displayed together with the progress status
    */
  label?: string;
}

It can be used to report progress for requests:

-> textDocument/references request (id 1)
<- window/progress notification (progress id 1, request id 1, status 0.5)
<- window/progress notification (progress id 1, request id 1, status 1)
<- textDocument/references response (id 1)

It can also be used to report progress for file indexing for example:

<- window/progress notification (progress id 2, status 1, total 1264, label "indexing file hello.php")
<- window/progress notification (progress id 2, status 134, total 1264, label "indexing file world.php")
<- window/progress notification (progress id 2, status 1264, total 1264, label "indexing file whatever.php")

A progress indicator should be hidden by the client once a progress with status == total comes in.

[kaloyan-raev]

Should we have some feedback from the client to server if it was able to process a certain progress status?

Imagine, like in the indexing, that the server floods the client with tons of progress status messages. If the system is not fast enough, we may have the situation where the server is notifying for progress status 1000 while the client still handles status 50 and has 950 more in the message queue.

The above situation can be mitigated if the server sends a new progress status only if the client was able to handle the previous one. This way the server will send only a fraction of all possible progress statuses, being gentle to the client.

[felixfbecker]

Well the notifications are like events, they don't get a response. The client can choose the handle them or not. In a way they are like UDP, we just send out how the progress is going. The server needs to make sure he only handles the newest one, similar like if you sent the current time over UDP every second, or doing VoIP. A client would not queue these, but just discard messages he could not handle.
Using a request/response scheme for this would be unneeded overhead for this, the server needs to generate a unique ID for every request and map the responses back once they come in. You avoid that with notifications, as there is no real "result" from the client in this case and the server is not interested in whether the client could handle the notification.

[felixfbecker]

Of course ClientCapabilities should be extended so a server can save sending these notifications if the client has no use for it.

[kaloyan-raev]

Although it is just a notification it can really cause a lot of overhead. Even just discarding the notifications can be expensive if the messages are thousands per second.

I like the idea of utilizing the ClientCapabilities. Perhaps, the client can tell the server about the granularity of the progress information that makes sense to be sent. For example, if the client displays the progress in percentages then no more than 100 notifications are really necessary (instead of tens of thousands).

[felixfbecker]

Sure, but using request/response would add even more overhead.
The number is not really important, it's the rate - in our use case, when we index 1000 files, I would send 1000 progress notifications, and that would be okay because it would be over a timespan of over a minute.
I doubt any server would have so granular progress indications for something simple like a references request that it would be a problem. For example, if the server knows he needs to iterate x ASTs, he can send a progress notification after every iterated AST. Basically I would shift that responsibility to the server, he should be responsible for not spamming notifications after every CPU tick (that would be bad for the server performance too, not just the client)

[kaloyan-raev]

Sure, but using request/response would add even more overhead.

If we can take advantage of the ClientCapabilities then request/response would not be necessary anymore.

The number is not really important, it's the rate - in our use case, when we index 1000 files, I would send 1000 progress notifications, and that would be okay because it would be over a timespan of over a minute.

It is exactly the rate that worries me. At the moment, on my machine, the PHP Language Server indexes ~ 150 files per seconds. It's too much to send a progress notification 150 times per second.

[felixfbecker]

It is exactly the rate that worries me. At the moment, on my machine, the PHP Language Server indexes ~ 150 files per seconds. It's too much to send a progress notification 150 times per second.

Actually, we already send those. We send a logMessage notification for each file. Haven't noticed any problems with this really, but we could easily choose to report only every 10th file or so.

[kaloyan-raev]

Actually, we already send those. We send a logMessage notification for each file. Haven't noticed any problems with this really, but we could easily choose to report only every 10th file or so.

I did some performance measurements. The test case was the PHP language server to index its own source code. The performance penalty of sending a logMessage for each file is around 3%. This is on the server side. There should be an additional performance penalty on the client side that I cannot measure right now.

We can have a long debate if this performance penalty is significant or not. For other use cases with other language servers, the performance penalty of such intensive progress reporting can be significantly lower or higher. Also, different users have different perception what performance penalty is acceptable or not.

This is why most software solutions make logging configurable. You can switch it and off. You can set the a log level (like only error messages, but no warnings and info messages). BTW, this is something we miss in the LSP - the client should be able to tell if logging is desired and at what level.

Same for progress reporting - the client should be able to tell if progress reporting is desired and at what rate. Some clients may want a progress progress not more frequently than 1 seconds, others than 100 ms, or 10 ms. There could be clients that want it all, or not at all.

It should not be a big deal to incorporate such progress reporting rate in the ClientCapablities and have servers respecting it.

[felixfbecker]

I did some performance measurements. The test case was the PHP language server to index its own source code. The performance penalty of sending a logMessage for each file is around 3%. This is on the server side

For the record, PHP is not the fastest, and my implementation probably isn't either.

Same for progress reporting - the client should be able to tell if progress reporting is desired and at what rate. Some clients may want a progress progress not more frequently than 1 seconds, others than 100 ms, or 10 ms. There could be clients that want it all, or not at all.

I'm all for having a flag in ClientCapabilities to enable/disable progress reports.

It should not be a big deal to incorporate such progress reporting rate in the ClientCapablities and have servers respecting it.

It complicates the implementation a bit. For basic progress reporting, we just have to increment a counter together and report it with total after each parse. With rate limiting, we have to hold a timestamp of the last progress report, measure if the difference, and only report if the difference is greater than the rate limit.

Proposal for ClientCapabilities in that case:

interface ClientCapabilities {
  /**
    * The client provides support for window/progress notifications
    * If a number, specifies a rate limit as the minimum number of milliseconds between two progress notifications.
    * If true, there is no rate limit.
    */
  progressProvider?: boolean | number;
}

[kaloyan-raev]

How about using an additional ProgressOptions interface like CompletionOptions is used for the completionProvider?

/**
 * Progress reporting options.
 */
interface ProgressOptions {
  /**
   * The minimum number of milliseconds between two progress notifications.
   */
  rateLimit?: number;
}

interface ClientCapabilities {
  /**
   * The client can process window/progress notifications.
   */
  progressProvider?: ProgressOptions;
}

This way it is easy to extend the protocol with additional options without breaking compatibility. It will also avoid the issue described in #39.

[felixfbecker]

So null or undefined would then mean no progress support

[kaloyan-raev]

It should be the same semantic like the providers in the ServerCapabilities.

null or undefined for the progressProvider would mean no progress support, i.e. the server should not send any progress notifications.

null or undefined for the rateLimit should mean no rate limit, i.e. the server should send all progress notifications without any limitation. Naturally same should happen if rateLimit is 0 or negative.

[bruno-medeiros]

It complicates the implementation a bit. For basic progress reporting, we just have to increment a counter together and report it with total after each parse. With rate limiting, we have to hold a timestamp of the last progress report, measure if the difference, and only report if the difference is greater than the rate limit.

That's only a few extra lines of code, it doesn't seem that much of a hurdle.

[martinring]

The rate limit makes no sense to me. It would have to be introduced in various other places as well after that logic. publishDiagnostics is far more complex and is not rate limited. It should be clear, that a language server is not to send hundreds of thousands progress messages per second, just as a language server does not send hundreds of thousands diagnostics per second.

Also introducing a rate limit as a static client capability seems a bad solution to me

• What happens if the server does not respect the rate limit?
• How would you even determine a static value for a rate limit. It depends on the hardware capabilities as well as other software running.

If something like that should be introduced I would rather suggest a global (symmetrical) back-pressure mechanism on the messaging level. But again that would make implementations more complicated.

As a last point:

There is no added value for a user if the rate of any message is above say 50 per second, since that is not observable for humans anymore. For any client that kind of message rate seems no problem at all so I would just suggest to add a design guideline for server implementations.

[felixfbecker]

I share @martinring's opinion. Servers should just treat the rate of sending notifications with common sense. In 99% of the cases this will probably not cause a problem and if it does an issue should be opened at the LS repository that it should overthink the points at which the notifications are sent.

[kaloyan-raev]

The rate limit makes no sense to me. It would have to be introduced in various other places as well after that logic. publishDiagnostics is far more complex and is not rate limited. It should be clear, that a language server is not to send hundreds of thousands progress messages per second, just as a language server does not send hundreds of thousands diagnostics per second.

Yes, publishDiagnostics is not rate limited. But it does not means we should not think about performance optimizations there too. @martinring, you are just assuming that language servers do not send hundreds and thousands of publishDiagnostics notifications per second to the client. But you don't really know. In fact, currently the PHP language server sends a publishDiagnostics notification whenever a PHP file is parsed. This means that during indexing there is a publishDiagnostics notification sent for each PHP file. These are another 150 messages per seconds in the pipe. Does it make any sense? Maybe. There are IDEs like Eclipse that want to display every possible diagnostic in a dedicated Problems view. But most of the other IDEs and editors are only interested in diagnostics for the files that are currently opened. How does the server know if the client is interested in diagnostics for all project files or only for the opened ones? It does not. Any assumption has a high chance to be wrong. The client should be able to tell the server what is expected.

Same for log messages that I mentioned in a previous comment. The client should be able to tell the server what is the expected log level of log messages.

Following the same logic, we should have some way to limit the number of progress notifications.

What happens if the server does not respect the rate limit?

Perhaps, nothing fatal would happen. Just the communication between the client and the server would be wasteful. This may result in noticeable issues with responsiveness in the IDE in certain situation. It may become more severe if the client and the server are not on the same host and networking between them is expensive.

How would you even determine a static value for a rate limit. It depends on the hardware capabilities as well as other software running.

I don't think it depends that much on the hardware capabilities rather than on the design goals of the client. Some clients would want to display the progress in a smooth way and they would want something like 30 progress updates per seconds, i.e. rate limit of 30 ms. I would expect most clients to want just a few refreshes per second, i.e. rate limit of 200-500 ms. On the other extreme would be clients that connect to the server over the wire and networking is very expensive. They would want to limit the the progress update to once every 3 seconds, i.e. rate limit 3000 ms.

As you can see there is 100 times difference between the two extremes. How could the server assume a fixed rate limit value that fits all client? Isn't it best to let the client tell it?

The LSP was designed after VSCode. There are lot of assumptions done that are closely related to the VSCode implementation. Most of issues opened are for clarifying these assumptions and even trying to get rid of some of them. The more clients try to adopt LSP, the more assumptions will be challenged.

I suggest to avoid making some assumption in this case. Let's not assume that progress notifications are cheap and that the client and the server are running on the same host.

[martinring]

@kaloyan-raev I agree, that publishDiagnostics should get more information about what information is desired, e.g. though view perspectives (right now the only information on the server side is which files are open, not which files are looked at and where). But this is completely off-topic.

A rate limit seems just a very bad soultion to me. I don't take your argument about the network connection since a server should adapt to slow network individually. (slow network might be a temporary condition) There exist established solutions for this. In this case messages could be combined (or dropped) to account for slow network, which would (for a reasonable server) result in less progress messages beeing sent. But again this is nothing you can point out up front.

Is there even a real problem anywhere? I think this should be discussed, when a client exists or is beeing developed which cannot handle more than x progress messages per second. (I cannot imagine that x is below say 100 even for the most inefficient implementations) Otherwise this discussion seem a little pointless.

[kaloyan-raev]

Is there even a real problem anywhere?

Yes, there is. Wasteful CPU utilization and responsiveness lag, which I observe with the PHP language server. In a previous comment I mentioned that I measured 3% performance penalty, because of the use case with 150 notifications per second. And this is not the most aggressive situation possible. Imagine a file copy or ZIP extract operation that reports a progress on each file processed. This can easily grow to thousands of progress notifications per second.

A full pipe with garbage notification delays the rest of messages, which hits responsiveness in the editor during operation with intensive progress reporting.

[felixfbecker]

I think this is somewhat premature optimization. We should add progress reporting to the protocol, and if any real performance issues occur, we can design a solution for it that fixes the problem for all requests in a universal way, not just progress. From our current LS the perf will not change, the method will just change from logMessage to progress.

[felixfbecker]

A full pipe with garbage notification delays the rest of messages, which hits responsiveness in the editor during operation with intensive progress reporting.

A client can keep seperate buffers for responses and notifications. Responses can have a higher priority. Requests are not delayed at the client because they are in a different stream.

[kaloyan-raev]

A client can keep seperate buffers for responses and notifications. Responses can have a higher priority. Requests are not delayed at the client because they are in a different stream.

The communication channel between the client and the server is still a single one. So the client needs to read and decode the message before determining if it is for the "responses" or "notifications" buffer. This will not solve the responsiveness issue. As I have already mentioned, even discarding unwanted messages is expensive.

Also, who says that requests are with higher priority than notifications? Yet another assumption. As I am typing in the editor, I would be more interested to see real-time diagnostics than real-time progress of some background operation. Both progress and diagnostics are notifications.

[martinring]

@kaloyan-raev your 3% performance penalty was measured on the server if I got you right. Why don't you just reduce the rate of progress messages in your server implementation. What is this to do with the client or the protocol?

[kaloyan-raev]

This was just an example measurement done in VS Code to back my thoughts with some real data. I am actually interested in other clients where each message has a higher cost.

I don't have my own language server implementation. I contribute to a couple of language server implementations and plan to consume even more. I can imagine tough discussions with each of the language servers for hardcoding the progress reporting to once or twice per second.

Hence it would be much better to have an established way for this client-server negotiation directly in the LSP.

[kdvolder]

Count me as a +1 for wanting a progress reporting mechanism. I like @felixfbecker proposal from Nov 4th the best. I prefer it over the ones proposed earlier because besides allowing for progress reports tied to a specific request, it also allows for reporting progress on background work, such as, for example building an index (which may not be directly linked to a specific client-side request).

Also agree with @martinring statements about ratelimiting. Any sensible server implementation should know its bad to flood the client with progress messages, so we don't need something in the protocol to tell the server that.

It does make sense to have ClientCapabilities to inform a server whether its capable of handling progress messages at all.

[vladdu]

Wouldn't a way to avoid the server drowning the client in notifications be to let the client request the status whenever it wants to update the UI?

[felixfbecker]

That would mean you use a request/response cycle (so you need request IDs) and the client would essentially poll the server. That means you will have request/response cycles with progress reports even though there was no progress, and also there may be progress that the client doesn't get to know until the next polling interval. You also would have no way to display arbitrary progress for async actions like indexing. Server-sent progress notifications are a better suited here.

[vladdu]

Yes, the client would poll the server but not necessarily at a fixed interval. The server need not answer until there is something to report, more or less like HTTP long-polling. The advantage is that the client has complete control over the traffic. Multiple such requests can be active at the same time, for different "channels" (one of which could be for "general async operations").

One disadvantage is that the client must become a server too (but there is already the showMessage request, so it's already required). Another is that the client becomes a bit more complex, but I think it's easier to handle than the risk of being swamped by notifications.

[dbaeumer]

Is someone willing to come up with a PR for this ? I add a capability flag mechanism to 3.0.2-beta.2 to be able to easier experiment with stuff like this.

[felixfbecker]

Will do. What exactly do you mean with capability flag machanism? Don't we already have that?

[ljw1004]

@felixfbecker Please expand more on this part of your proposal:

/** a unique identifier to associate multiple progress notifications with the same progress */
  id: string | number;

Can you give a concrete example scenario where you use differend ids? What does "the same progress" mean conceptually?

I can't imagine how it would ever be used. I can see value in reporting progress for a given request (in which case requestId would be used). And I can see value in reporting a global progress notification in the status bar (in our case, when a file is changed on disk then the language-server is busy for 1-2 seconds to digest what has happened and the user should see a spinner). But I can't think how notifications with different identifiers would be surfaced in the UI. So: I'd be inclined to remove this field, and rename your requestId to be just id.

[felixfbecker]

@ljw1004 That is meant for progress bars not associated with a request, like indexing, type checking, ...
It's of course also possible to only support a single global progress bar that is not associated to requests.

This proposal also predates the proposal for streaming #182, it would probably cause a lot of overhead to send both partial results _and_ progress notifications. Maybe progress should be included as an optional field in a partial result (which leaves the question open how to report progress not associated with a request).

[ljw1004]

@felixfbecker Please explain more. Why would you have two different ids? Are you envisaging two separate progress bars on-screen at once? Or, as many different on-screen progress bars as there are id numbers?

[felixfbecker]

My thought behind it was the use case of reporting progress for operations that don't belong to a request. For example, indexing, type checking, linting etc that can all be done in the background, triggered on initialize or a notification like textDocument/didChange. A client _could_ then display these eg as multiple progress bars in the status bar.

[ljw1004]

I see. So your concrete example works like this:

// User does an edit
{id: 'lint', status: 0}
{id: 'index', status: 0}
{id: 'typecheck', status: 0}

// At this point the UI displays a spinner, and if the user
// hovers over it, then a tooltip displays "lint: 0%, index: 0%, typecheck: 0%"

{id: 'lint', status: 0.5}

// At this point the tooltip displays "lint: 50%, index: 0%, typecheck: 0%"

{id: 'typecheck', status: 1}

// At this point status == total, so by definition this progress id has completed.
// so the tooltip displays "lint: 50%, index: 0%"

{id: 'index', status: 1}

// At this point the tooltip displays "lint: 50%"

{id: 'lint', status: 0.7}

// At this point the tooltip displays "lint: 70%"

{id: 'lint', status: 1}

// At this point the spinner disappears from the status bar.

Is that what you have in mind?

[felixfbecker]

Yeah, exactly. Except that of course instead of spinner/tooltip, it could also show multiple progress bars next to each other, or stacked or little pie charts or whatever.

[daviwil]

Multiple progress bars would be extremely helpful for PowerShell, we have a progress reporting API that enables reporting progress on subtasks of a larger task which has its own progress. Having multiple progress bars would allow me to emulate that.

[ljw1004]

Then I think we should have a crisper notion of "when something is completed" because that has a solid UI effect (e.g. causing a progress bar to disappear).

It would be bad if something like a floating point rounding area meant that something only ever got a status of 99.99%.

[keegancsmith]

For people following along I sent out a PR implementing a very minimal window/progress spec #245. It was inspired by what is in this issue and the newly available progress API for vscode extensions. It is currently marked as closed, but if others are interested in it it may be appropriate to re-open.

[Xanewok]

To spark/further current discussion on the feature, I pushed a PR with @keegancsmith's proposed protocol extension and a reference client implementation for it in https://github.com/Microsoft/vscode-languageserver-node/pull/261.
Please tell me your thoughts, also how to improve upon it or maybe if it'd be feasible to include it in the protocol itself.

[dbaeumer]

I merged in @Xanewok work for progress into the vscode libraries as a proposed API. While implementing cancel support I separated the notification into start, report, done and cancel. Mainly because it was very hard to descibe in which cases which property combinations were valid. Was a lot easier having separate literals per request. The currently implemented proposal looks like this:

Reporting server task progress

Many tools are capable of performing some background task processing or data streaming. From a UX point of view, it's good to report both the fact that the tool is performing some background work, but also report the progress being made for it. To realize that and to provide a simple proposal based on which the feature can be later improved, the following additions are proposed:

_Client Capabilities_:

The client sets the following capability if it is supporting notifying task progress.

    /**
     * Window specific client capabilities.
     */
    window?: {
        /**
         * Whether client supports handling progress notifications.
         */
        progress?: boolean;
    }

Progress Start Notification

The window/progress/start notification is sent from the server to the client to ask the client to start progress.

_Notification_:

• method: 'window/progress/start'
• params: ProgressStartParams defined as follows:

export interface ProgressStartParams {

    /**
     * A unique identifier to associate multiple progress notifications with
     * the same progress.
     */
    id: string;

    /**
     * Mandatory title of the progress operation. Used to briefly inform about
     * the kind of operation being performed.
     *
     * Examples: "Indexing" or "Linking dependencies".
     */
    title: string;

    /**
     * Controls if a cancel button should show to allow the user to cancel the
     * long running operation. Clients that don't support cancellation are allowed
     * to ignore the setting.
     */
    cancellable?: boolean;

    /**
     * Optional, more detailed associated progress message. Contains
     * complementary information to the `title`.
     *
     * Examples: "3/25 files", "project/src/module2", "node_modules/some_dep".
     * If unset, the previous progress message (if any) is still valid.
     */
    message?: string;

    /**
     * Optional progress percentage to display (value 100 is considered 100%).
     * If not provided infinite progress is assumed and clients are allowed
     * to ignore the `percentage` value in subsequent in report notifications.
     *
     * The value should be steadily rising. Clients are free to ignore values
     * that are not following this rule.
     */
    percentage?: number;
}

Progress Report Notification

The window/progress/report notification is sent from the server to the client to report progress for a previously started progress.

_Notification_:

• method: 'window/progress/report'
• params: ProgressReportParams defined as follows:

export interface ProgressReportParams {

    /**
     * A unique identifier to associate multiple progress notifications with the same progress.
     */
    id: string;

    /**
     * Optional, more detailed associated progress message. Contains
     * complementary information to the `title`.
     *
     * Examples: "3/25 files", "project/src/module2", "node_modules/some_dep".
     * If unset, the previous progress message (if any) is still valid.
     */
    message?: string;

    /**
     * Optional progress percentage to display (value 100 is considered 100%).
     * If infinite progress was indicated in the start notification client
     * are allowed to ignore the value. In addition the value should be steadily
     * rising. Clients are free to ignore values that are not following this rule.
     */
    percentage?: number;
}

Progress Done Notification

The window/progress/done notification is sent from the server to the client to stop a previously started progress.

_Notification_:

• method: 'window/progress/done'
• params: ProgressDoneParams defined as follows:

export interface ProgressDoneParams {
    /**
     * A unique identifier to associate multiple progress notifications with the same progress.
     */
    id: string;
}

Progress Cancel Notification

The window/progress/cancel notification is sent from the client to the server to inform the server that the user has pressed the
cancel button on the progress UX. A server receiving a cancel request must still close a progress using the done notification.

_Notification_:

• method: 'window/progress/cancel'
• params: ProgressCancelParams defined as follows:

export interface ProgressCancelParams {
    /**
     * A unique identifier to associate multiple progress notifications with the same progress.
     */
    id: string;
}

[ljw1004]

@dbaumer Should we be using this progress mechanism for the LSP server to indicate a long-running process of generating diagnostics? Or should that await some future LSP message?

Context: Users have a keen wish to know when a long-running diagnostic-generating operation has finished. I imagine that progress-of-computing-diagnostics would naturally be surfaced in the UI with in the "Issues" tab itself, or within the "issues count" icons that appear in the status bar. I don't know if you expect these progress messages to typically be surfaced elsewhere...

[dbaeumer]

@ljw1004 since publish diagnostics is a push model the server should make use of the new API if wanted. If we find out that the server needs to define a location we can add something later on.

What I am also working on is to support progress per request so that the client can upfront decide where to best render the UI.

[dbaeumer]

I published next versions of the VS Code LSP protocol, client and server libs that have support for progress.

[dbaeumer]

Please see also https://github.com/microsoft/language-server-protocol/issues/786

[dbaeumer]

Closing the issue. Corresponding support has been speced in dbaeumer/3.15 branch.