Performance optimization

[Kaidesuyoo]

Hello VSCode team.
I've made some performance optimizations and the most impactful change is here:

// before
const cancellationTokenListener = cancellationToken.onCancellationRequested(cancel);
disposable = combinedDisposable(toDisposable(cancel), cancellationTokenListener);

//after
disposable = cancellationToken.onCancellationRequested(cancel);

Why?

Before the modification, the cancel function was added to the disposable. In my understanding, this cancel function should be used to promptly cancel the request when a cancellation occurs.
However, when the request has not been canceled and a response is received, the result promise reaches the finally phase:

return result.finally(() => {
	// This will result in the cancel function being executed regardless.
	disposable.dispose();
	this.activeRequests.delete(disposable);
});

This is what the cancel function does:

const cancel = () => {
	if (uninitializedPromise) {
		uninitializedPromise.cancel();
		uninitializedPromise = null;
	} else {
		// After the client connects, a cancellation request is sent after each request is completed.
		this.sendRequest({ id, type: RequestType.PromiseCancel });
	}
	e(new CancellationError());
};

On the server side, the received requests are cleaned up after processing, meaning that each cancellation request triggered by the client results in an unnecessary cleanup on the server.

// server side
finally(() => {
	disposable.dispose();
	// Delete after processing the request.
	this.activeRequests.delete(request.id);
});

// After the server returns the processed request, it receives a cancellation request from the client.
private disposeActiveRequest(request: IRawRequest): void {
	const disposable = this.activeRequests.get(request.id);
	// The request has already been processed and deleted, so the following code will not execute.
	if (disposable) {
		disposable.dispose();
		this.activeRequests.delete(request.id);
	}
}

In summary, I believe that the cancellation requests here are meaningless and occur almost for every request established through IPC communication.

This situation started occurring after this change.

[Kaidesuyoo]

@microsoft-github-policy-service agree

[connor4312]

I'm not sure how your PR description relates to the code you changed. Your code is in requestEvent but you refer to processing in requestPromise.

Cancellation is not requested when disposing a CancellationTokenSource by default so I'm not sure that I see where the problem is occurring -- were you able to verify what you say actually happens when debugging the server?

[Kaidesuyoo]

I'm not sure how your PR description relates to the code you changed. Your code is in requestEvent but you refer to processing in requestPromise.

Cancellation is not requested when disposing a CancellationTokenSource by default so I'm not sure that I see where the problem is occurring -- were you able to verify what you say actually happens when debugging the server?

I apologize for the confusion. The code in requestEvent is a different part; what I described in the PR pertains to the code within requestPromise.

As you mentioned, disposing of the CancellationTokenSource does not trigger the cancellation request. The issue I'm referring to is that the cancel function is added to the disposable, which causes the cancel function to be triggered upon disposal.

Here’s what happened during debugging:

debug.mp4

[connor4312]

Thanks for the PR!