Merge pull request #4310 from an-dr-eas-k/master

improve queues, (one effect: improve code completion)

Author: JoeRobich

src/observers/OmnisharpDebugModeLoggerObserver.ts

@@ -5,7 +5,7 @@
 
 import { BaseLoggerObserver } from "./BaseLoggerObserver";
 import * as os from 'os';
-import { BaseEvent, OmnisharpRequestMessage, OmnisharpServerEnqueueRequest, OmnisharpServerDequeueRequest, OmnisharpServerVerboseMessage, OmnisharpServerProcessRequestStart, OmnisharpEventPacketReceived } from "../omnisharp/loggingEvents";
+import { BaseEvent, OmnisharpRequestMessage, OmnisharpServerEnqueueRequest, OmnisharpServerDequeueRequest, OmnisharpServerRequestCancelled, OmnisharpServerVerboseMessage, OmnisharpServerProcessRequestStart, OmnisharpEventPacketReceived } from "../omnisharp/loggingEvents";
 import { EventType } from "../omnisharp/EventType";
 
 export class OmnisharpDebugModeLoggerObserver extends BaseLoggerObserver {
@@ -20,6 +20,9 @@ export class OmnisharpDebugModeLoggerObserver extends BaseLoggerObserver {
             case EventType.OmnisharpServerDequeueRequest:
                 this.handleOmnisharpServerDequeueRequest(<OmnisharpServerDequeueRequest>event);
                 break;
+            case EventType.OmnisharpServerRequestCancelled:
+                this.handleOmnisharpServerRequestCancelled(<OmnisharpServerRequestCancelled>event);
+                break;
             case EventType.OmnisharpServerProcessRequestStart:
                 this.handleOmnisharpProcessRequestStart(<OmnisharpServerProcessRequestStart>event);
                 break;
@@ -44,17 +47,22 @@ export class OmnisharpDebugModeLoggerObserver extends BaseLoggerObserver {
     }
 
     private handleOmnisharpServerEnqueueRequest(event: OmnisharpServerEnqueueRequest) {
-        this.logger.appendLine(`Enqueue ${event.name} request for ${event.command}.`);
+        this.logger.appendLine(`Enqueue to ${event.queueName} request for ${event.command}.`);
         this.logger.appendLine();
     }
 
     private handleOmnisharpServerDequeueRequest(event: OmnisharpServerDequeueRequest) {
-        this.logger.appendLine(`Dequeue ${event.name} request for ${event.command} (${event.id}).`);
+        this.logger.appendLine(`Dequeue from ${event.queueName} with status ${event.queueStatus} request for ${event.command}${event.id ? ` (${event.id})` : ''}.`);
+        this.logger.appendLine();
+    }
+
+    private handleOmnisharpServerRequestCancelled(event: OmnisharpServerRequestCancelled) {
+        this.logger.appendLine(`Cancelled request for ${event.command} (${event.id}).`);
         this.logger.appendLine();
     }
 
     private handleOmnisharpProcessRequestStart(event: OmnisharpServerProcessRequestStart) {
-        this.logger.appendLine(`Processing ${event.name} queue`);
+        this.logger.appendLine(`Processing ${event.name} queue, available slots ${event.availableRequestSlots}`);
         this.logger.increaseIndent();
     }
 

src/omnisharp/EventType.ts

@@ -82,7 +82,8 @@ export enum EventType {
     ProjectDiagnosticStatus = 75,
     DotNetTestRunInContextStart = 76,
     DotNetTestDebugInContextStart = 77,
-    TelemetryErrorEvent = 78
+    TelemetryErrorEvent = 78,
+    OmnisharpServerRequestCancelled = 79,
 }
 
 //Note that the EventType protocol is shared with Razor.VSCode and the numbers here should not be altered

src/omnisharp/loggingEvents.ts

@@ -115,17 +115,22 @@ export class OmnisharpServerUnresolvedDependencies implements BaseEvent {
 
 export class OmnisharpServerEnqueueRequest implements BaseEvent {
     type = EventType.OmnisharpServerEnqueueRequest;
-    constructor(public name: string, public command: string) { }
+    constructor(public queueName: string, public command: string) { }
 }
 
 export class OmnisharpServerDequeueRequest implements BaseEvent {
     type = EventType.OmnisharpServerDequeueRequest;
-    constructor(public name: string, public command: string, public id: number) { }
+    constructor(public queueName: string, public queueStatus: string, public command: string, public id?: number) { }
+}
+
+export class OmnisharpServerRequestCancelled implements BaseEvent {
+    type = EventType.OmnisharpServerRequestCancelled;
+    constructor(public command: string, public id: number) { }
 }
 
 export class OmnisharpServerProcessRequestStart implements BaseEvent {
     type = EventType.OmnisharpServerProcessRequestStart;
-    constructor(public name: string) { }
+    constructor(public name: string, public availableRequestSlots: number) { }
 }
 
 export class OmnisharpEventPacketReceived implements BaseEvent {

src/omnisharp/prioritization.ts

@@ -13,7 +13,8 @@ const priorityCommands = [
 ];
 
 const normalCommands = [
-    protocol.Requests.AutoComplete,
+    protocol.Requests.Completion,
+    protocol.Requests.CompletionResolve,
     protocol.Requests.FilesChanged,
     protocol.Requests.FindSymbols,
     protocol.Requests.FindUsages,

src/omnisharp/protocol.ts

@@ -8,7 +8,6 @@ import { CompletionTriggerKind, CompletionItemKind, CompletionItemTag, InsertTex
 
 export module Requests {
     export const AddToProject = '/addtoproject';
-    export const AutoComplete = '/autocomplete';
     export const CodeCheck = '/codecheck';
     export const CodeFormat = '/codeformat';
     export const ChangeBuffer = '/changebuffer';

src/omnisharp/requestQueue.ts

@@ -14,6 +14,7 @@ export interface Request {
     onError(err: any): void;
     startTime?: number;
     endTime?: number;
+    id?: number;
 }
 
 /**
@@ -47,7 +48,7 @@ class RequestQueue {
 
         if (request) {
             this._waiting.delete(id);
-            this.eventStream.post(new OmnisharpServerDequeueRequest(this._name, request.command, id));
+            this.eventStream.post(new OmnisharpServerDequeueRequest(this._name, "waiting", request.command, id));
         }
 
         return request;
@@ -57,12 +58,12 @@ class RequestQueue {
         let index = this._pending.indexOf(request);
         if (index !== -1) {
             this._pending.splice(index, 1);
-
-            // Note: This calls reject() on the promise returned by OmniSharpServer.makeRequest
-            request.onError(new Error(`Pending request cancelled: ${request.command}`));
+            this.eventStream.post(new OmnisharpServerDequeueRequest(this._name, "pending", request.command));
         }
 
-        // TODO: Handle cancellation of a request already waiting on the OmniSharp server.
+        if (request.id){
+            this.dequeue(request.id);
+        }
     }
 
     /**
@@ -87,11 +88,10 @@ class RequestQueue {
             return;
         }
 
-        this.eventStream.post(new OmnisharpServerProcessRequestStart(this._name));
-
-        const slots = this._maxSize - this._waiting.size;
+        const availableRequestSlots = this._maxSize - this._waiting.size;
+        this.eventStream.post(new OmnisharpServerProcessRequestStart(this._name, availableRequestSlots));
 
-        for (let i = 0; i < slots && this._pending.length > 0; i++) {
+        for (let i = 0; i < availableRequestSlots && this._pending.length > 0; i++) {
             const item = this._pending.shift();
             item.startTime = Date.now();
 

src/omnisharp/server.ts

@@ -572,7 +572,10 @@ export class OmniSharpServer {
 
         if (token) {
             token.onCancellationRequested(() => {
+                this.eventStream.post(new ObservableEvents.OmnisharpServerRequestCancelled(request.command, request.id));
                 this._requestQueue.cancelRequest(request);
+                // Note: This calls reject() on the promise returned by OmniSharpServer.makeRequest
+                request.onError(new Error(`Request ${request.command} cancelled, id: ${request.id}`));
             });
         }
 
@@ -705,6 +708,7 @@ export class OmniSharpServer {
 
     private _makeRequest(request: Request) {
         const id = OmniSharpServer._nextId++;
+        request.id = id;
 
         const requestPacket: protocol.WireProtocol.RequestPacket = {
             Type: 'request',

test/unitTests/logging/OmnisharpDebugModeLoggerObserver.test.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 import { use, should, expect } from 'chai';
 import { getNullChannel } from '../testAssets/Fakes';
-import { OmnisharpServerVerboseMessage, EventWithMessage, OmnisharpRequestMessage, OmnisharpServerEnqueueRequest, OmnisharpServerDequeueRequest, OmnisharpServerProcessRequestStart, OmnisharpEventPacketReceived, OmnisharpServerProcessRequestComplete } from '../../../src/omnisharp/loggingEvents';
+import { OmnisharpServerVerboseMessage, EventWithMessage, OmnisharpRequestMessage, OmnisharpServerEnqueueRequest, OmnisharpServerDequeueRequest, OmnisharpServerProcessRequestStart, OmnisharpEventPacketReceived, OmnisharpServerProcessRequestComplete, OmnisharpServerRequestCancelled } from '../../../src/omnisharp/loggingEvents';
 import { OmnisharpDebugModeLoggerObserver } from '../../../src/observers/OmnisharpDebugModeLoggerObserver';
 
 use(require("chai-string"));
@@ -33,27 +33,36 @@ suite("OmnisharpDebugModeLoggerObserver", () => {
     test(`OmnisharpServerEnqueueRequest: Name and Command is logged`, () => {
         let event = new OmnisharpServerEnqueueRequest("foo", "someCommand");
         observer.post(event);
-        expect(logOutput).to.contain(event.name);
+        expect(logOutput).to.contain(event.queueName);
         expect(logOutput).to.contain(event.command);
     });
 
-    test(`OmnisharpServerDequeueRequest: Name and Command is logged`, () => {
-        let event = new OmnisharpServerDequeueRequest("foo", "someCommand", 1);
+    test(`OmnisharpServerDequeueRequest: QueueName, QueueStatus, Command and Id is logged`, () => {
+        let event = new OmnisharpServerDequeueRequest("foo", "pending", "someCommand", 1);
         observer.post(event);
-        expect(logOutput).to.contain(event.name);
+        expect(logOutput).to.contain(event.queueName);
+        expect(logOutput).to.contain(event.queueStatus);
         expect(logOutput).to.contain(event.command);
         expect(logOutput).to.contain(event.id);
     });
 
-    test(`OmnisharpProcessRequestStart: Name is logged`, () => {
-        let event = new OmnisharpServerProcessRequestStart("foobar");
+    test(`OmnisharpProcessRequestStart: Name and slots is logged`, () => {
+        let event = new OmnisharpServerProcessRequestStart("foobar", 2);
         observer.post(event);
         expect(logOutput).to.contain(event.name);
+        expect(logOutput).to.contain(event.availableRequestSlots);
+    });
+
+    test(`OmnisharpServerRequestCancelled: Name and Id is logged`, () => {
+        let event = new OmnisharpServerRequestCancelled("foobar", 23);
+        observer.post(event);
+        expect(logOutput).to.contain(event.command);
+        expect(logOutput).to.contain(event.id);
     });
 
     test(`OmnisharpServer messages increase and decrease indent`, () => {
         observer.post(new OmnisharpServerVerboseMessage("!indented_1"));
-        observer.post(new OmnisharpServerProcessRequestStart("name"));
+        observer.post(new OmnisharpServerProcessRequestStart("name", 2));
         observer.post(new OmnisharpServerVerboseMessage("indented"));
         observer.post(new OmnisharpServerProcessRequestComplete());
         observer.post(new OmnisharpServerVerboseMessage("!indented_2"));