From 70f39e04a21610f1b1259c15cae057876f046944 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Tue, 8 Nov 2016 15:25:05 -0800 Subject: [PATCH 1/9] Add queue helper classes for handling requests concurrently --- src/omnisharp/prioritization.ts | 60 +++++++++++++++ src/omnisharp/protocol.ts | 26 +++++++ src/omnisharp/queue.ts | 131 ++++++++++++++++++++++++++++++++ src/omnisharp/server.ts | 74 +++++++----------- 4 files changed, 245 insertions(+), 46 deletions(-) create mode 100644 src/omnisharp/prioritization.ts create mode 100644 src/omnisharp/queue.ts diff --git a/src/omnisharp/prioritization.ts b/src/omnisharp/prioritization.ts new file mode 100644 index 0000000..98251b3 --- /dev/null +++ b/src/omnisharp/prioritization.ts @@ -0,0 +1,60 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as protocol from './protocol'; + +const priorityCommands = [ + protocol.Requests.ChangeBuffer, + protocol.Requests.FormatAfterKeystroke, + protocol.Requests.FormatRange, + protocol.Requests.UpdateBuffer +]; + +const normalCommands = [ + protocol.Requests.AutoComplete, + protocol.Requests.FilesChanged, + protocol.Requests.FindSymbols, + protocol.Requests.FindUsages, + protocol.Requests.GetCodeActions, + protocol.Requests.GoToDefinition, + protocol.Requests.RunCodeAction, + protocol.Requests.SignatureHelp, + protocol.Requests.TypeLookup +]; + +const prioritySet = new Set(priorityCommands); +const normalSet = new Set(normalCommands); +const deferredSet = new Set(); + +const nonDeferredSet = new Set(); + +for (let command of priorityCommands) { + nonDeferredSet.add(command); +} + +for (let command of normalCommands) { + nonDeferredSet.add(command); +} + +export function isPriorityCommand(command: string) { + return prioritySet.has(command); +} + +export function isNormalCommand(command: string) { + return normalSet.has(command); +} + +export function isDeferredCommand(command: string) { + if (deferredSet.has(command)) { + return true; + } + + if (nonDeferredSet.has(command)) { + return false; + } + + deferredSet.add(command); + return true; +} \ No newline at end of file diff --git a/src/omnisharp/protocol.ts b/src/omnisharp/protocol.ts index de2ca1d..a0be3e2 100644 --- a/src/omnisharp/protocol.ts +++ b/src/omnisharp/protocol.ts @@ -29,6 +29,32 @@ export module Requests { export const Metadata = '/metadata'; } +export namespace WireProtocol { + export interface Packet { + Type: string; + Seq: number; + } + + export interface RequestPacket extends Packet { + Command: string; + Arguments: any; + } + + export interface ResponsePacket extends Packet { + Command: string; + Request_seq: number; + Running: boolean; + Success: boolean; + Message: string; + Body: any; + } + + export interface EventPacket extends Packet { + Event: string; + Body: any; + } +} + export interface Request { Filename: string; Line?: number; diff --git a/src/omnisharp/queue.ts b/src/omnisharp/queue.ts new file mode 100644 index 0000000..7608d14 --- /dev/null +++ b/src/omnisharp/queue.ts @@ -0,0 +1,131 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Logger } from '../logger'; +import * as protocol from './protocol'; +import * as prioritization from './prioritization'; + +interface Request { + path: string; + data?: any; + onSuccess(value: any): void; + onError(err: any): void; +} + +class RequestQueue { + private _pending: Request[] = []; + private _requests: Map = new Map(); + + public constructor( + private _maxSize: number, + private _logger: Logger, + private _makeRequest: (request: Request) => protocol.WireProtocol.RequestPacket) { + } + + public enqueue(request: Request) { + this._pending.push(request); + } + + public remove(command: string, seq: number) { + if (!this._requests.delete(seq)) { + this._logger.appendLine(`Received response for ${command} but could not find request.`); + } + } + + public hasPending() { + return this._pending.length === 0; + } + + public isFull() { + return this._requests.size >= this._maxSize; + } + + public drain() { + let i = 0; + const slots = this._maxSize - this._requests.size; + + do { + const item = this._pending.shift(); + const request = this._makeRequest(item); + this._requests.set(request.Seq, request); + + if (this.isFull()) { + return; + } + } + while (this._pending.length > 0 && ++i < slots) + } +} + +export class Queue { + private _isProcessing: boolean; + private _priorityQueue: RequestQueue; + private _normalQueue: RequestQueue; + private _deferredQueue: RequestQueue; + + public constructor( + logger: Logger, + concurrency: number, + makeRequest: (request: Request) => protocol.WireProtocol.RequestPacket + ) { + this._priorityQueue = new RequestQueue(1, logger, makeRequest); + this._normalQueue = new RequestQueue(concurrency, logger, makeRequest); + this._deferredQueue = new RequestQueue(Math.max(Math.floor(concurrency / 4), 2), logger, makeRequest); + } + + private getQueue(command: string) { + if (prioritization.isPriorityCommand(command)) { + return this._priorityQueue; + } + else if (prioritization.isNormalCommand(command)) { + return this._normalQueue; + } + else { + return this._deferredQueue; + } + } + + public enqueue(request: Request) { + const queue = this.getQueue(request.path); + queue.enqueue(request); + + this.drain(); + } + + public remove(command: string, seq: number) { + const queue = this.getQueue(command); + queue.remove(command, seq); + } + + private drain() { + if (this._isProcessing) { + return false; + } + + if (this._priorityQueue.isFull()) { + return false; + } + + if (this._normalQueue.isFull() && this._deferredQueue.isFull()) { + return false; + } + + if (this._priorityQueue.hasPending()) { + this._priorityQueue.drain(); + this._isProcessing = false; + return; + } + + if (this._normalQueue.hasPending()) { + this._normalQueue.drain(); + } + + if (this._deferredQueue.hasPending()) { + this._deferredQueue.drain(); + } + + this._isProcessing = false; + } +} \ No newline at end of file diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index e5606ea..2f8d9cc 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -399,6 +399,8 @@ export abstract class OmnisharpServer { return Promise.reject('server has been stopped or not started'); } + console.log(`makeRequest: path=${path}`); + let startTime: number; let request: Request; @@ -442,7 +444,6 @@ export abstract class OmnisharpServer { } private _processQueue(): void { - if (this._queue.length === 0) { // nothing to do this._isProcessingQueue = false; @@ -469,39 +470,13 @@ export abstract class OmnisharpServer { protected abstract _makeNextRequest(path: string, data: any): Promise; } -namespace WireProtocol { - - export interface Packet { - Type: string; - Seq: number; - } - - export interface RequestPacket extends Packet { - Command: string; - Arguments: any; - } - - export interface ResponsePacket extends Packet { - Command: string; - Request_seq: number; - Running: boolean; - Success: boolean; - Message: string; - Body: any; - } - - export interface EventPacket extends Packet { - Event: string; - Body: any; - } -} - export class StdioOmnisharpServer extends OmnisharpServer { - private static _seqPool = 1; + private static _nextId = 1; + private static StartupTimeout = 1000 * 60; - private _rl: ReadLine; - private _activeRequest: { [seq: number]: { onSuccess: Function; onError: Function; } } = Object.create(null); + private _readLine: ReadLine; + private _activeRequests: { [seq: number]: { onSuccess: Function; onError: Function; } } = Object.create(null); private _callOnStop: Function[] = []; constructor(reporter: TelemetryReporter) { @@ -513,7 +488,8 @@ export class StdioOmnisharpServer extends OmnisharpServer { public stop(): Promise { while (this._callOnStop.length) { - this._callOnStop.pop()(); + let method = this._callOnStop.pop(); + method(); } return super.stop(); @@ -525,7 +501,7 @@ export class StdioOmnisharpServer extends OmnisharpServer { this._fireEvent('stderr', String(data)); }); - this._rl = createInterface({ + this._readLine = createInterface({ input: this._serverProcess.stdout, output: this._serverProcess.stdin, terminal: false @@ -569,7 +545,7 @@ export class StdioOmnisharpServer extends OmnisharpServer { return; } - let packet: WireProtocol.Packet; + let packet: protocol.WireProtocol.Packet; try { packet = JSON.parse(line); } @@ -585,10 +561,10 @@ export class StdioOmnisharpServer extends OmnisharpServer { switch (packet.Type) { case 'response': - this._handleResponsePacket(packet); + this._handleResponsePacket(packet); break; case 'event': - this._handleEventPacket(packet); + this._handleEventPacket(packet); break; default: console.warn('unknown packet: ', packet); @@ -596,21 +572,23 @@ export class StdioOmnisharpServer extends OmnisharpServer { } }; - this._rl.addListener('line', onLineReceived); - this._callOnStop.push(() => this._rl.removeListener('line', onLineReceived)); + this._readLine.addListener('line', onLineReceived); + this._callOnStop.push(() => this._readLine.removeListener('line', onLineReceived)); } - private _handleResponsePacket(packet: WireProtocol.ResponsePacket): void { + private _handleResponsePacket(packet: protocol.WireProtocol.ResponsePacket): void { - const requestSeq = packet.Request_seq, - entry = this._activeRequest[requestSeq]; + const id = packet.Request_seq; + const entry = this._activeRequests[id]; if (!entry) { console.warn('Received a response WITHOUT a request', packet); return; } - delete this._activeRequest[requestSeq]; + console.log(`Handling response: ${packet.Command} (${id})`); + + delete this._activeRequests[id]; if (packet.Success) { entry.onSuccess(packet.Body); @@ -619,7 +597,7 @@ export class StdioOmnisharpServer extends OmnisharpServer { } } - private _handleEventPacket(packet: WireProtocol.EventPacket): void { + private _handleEventPacket(packet: protocol.WireProtocol.EventPacket): void { if (packet.Event === 'log') { // handle log events @@ -634,16 +612,20 @@ export class StdioOmnisharpServer extends OmnisharpServer { protected _makeNextRequest(path: string, data: any): Promise { - const thisRequestPacket: WireProtocol.RequestPacket = { + const id = StdioOmnisharpServer._nextId++; + + const thisRequestPacket: protocol.WireProtocol.RequestPacket = { Type: 'request', - Seq: StdioOmnisharpServer._seqPool++, + Seq: id, Command: path, Arguments: data }; + console.log(`Making request: ${path} (${id})`); + return new Promise((resolve, reject) => { - this._activeRequest[thisRequestPacket.Seq] = { + this._activeRequests[thisRequestPacket.Seq] = { onSuccess: value => resolve(value), onError: err => reject(err) }; From 66ecc01cca30e7cbf982490e1e0fa3ceafe81353 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Tue, 8 Nov 2016 15:35:24 -0800 Subject: [PATCH 2/9] Combine OmnisharpServer and StdioOmnisharpServer --- src/omnisharp/extension.ts | 4 +-- src/omnisharp/server.ts | 61 +++++++++++++------------------------- 2 files changed, 22 insertions(+), 43 deletions(-) diff --git a/src/omnisharp/extension.ts b/src/omnisharp/extension.ts index c0a2dbe..0839da6 100644 --- a/src/omnisharp/extension.ts +++ b/src/omnisharp/extension.ts @@ -23,7 +23,7 @@ import SignatureHelpProvider from '../features/signatureHelpProvider'; import registerCommands from '../features/commands'; import forwardChanges from '../features/changeForwarding'; import reportStatus from '../features/status'; -import {StdioOmnisharpServer} from './server'; +import {OmnisharpServer} from './server'; import {Options} from './options'; import {addAssetsIfNecessary, AddAssetResult} from '../assets'; @@ -33,7 +33,7 @@ export function activate(context: vscode.ExtensionContext, reporter: TelemetryRe scheme: 'file' // only files from disk }; - const server = new StdioOmnisharpServer(reporter); + const server = new OmnisharpServer(reporter); const advisor = new Advisor(server); // create before server is started const disposables: vscode.Disposable[] = []; const localDisposables: vscode.Disposable[] = []; diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index 2f8d9cc..119ab27 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -64,7 +64,14 @@ module Events { const TelemetryReportingDelay = 2 * 60 * 1000; // two minutes -export abstract class OmnisharpServer { +export class OmnisharpServer { + + private static _nextId = 1; + private static StartupTimeout = 1000 * 60; + + private _readLine: ReadLine; + private _activeRequests: { [seq: number]: { onSuccess: Function; onError: Function; } } = Object.create(null); + private _callOnStop: Function[] = []; private _reporter: TelemetryReporter; private _delayTrackers: { [requestName: string]: DelayTracker }; @@ -76,16 +83,14 @@ export abstract class OmnisharpServer { private _queue: Request[] = []; private _isProcessingQueue = false; private _channel: vscode.OutputChannel; - protected _logger: Logger; + private _logger: Logger; private _isDebugEnable: boolean = false; - protected _serverProcess: ChildProcess; - protected _extraArgs: string[]; - protected _options: Options; + private _serverProcess: ChildProcess; + private _options: Options; constructor(reporter: TelemetryReporter) { - this._extraArgs = []; this._reporter = reporter; this._channel = vscode.window.createOutputChannel('OmniSharp Log'); @@ -237,6 +242,7 @@ export abstract class OmnisharpServer { let args = [ '-s', solutionPath, '--hostPID', process.pid.toString(), + '--stdio', 'DotNet:enablePackageRestore=false', '--encoding', 'utf-8' ]; @@ -247,8 +253,6 @@ export abstract class OmnisharpServer { args.push('-v'); } - args = args.concat(this._extraArgs); - this._logger.appendLine(`Starting OmniSharp server at ${new Date().toLocaleString()}`); this._logger.increaseIndent(); this._logger.appendLine(`Target: ${solutionPath}`); @@ -295,10 +299,13 @@ export abstract class OmnisharpServer { }); } - protected abstract _doConnect(): Promise; - public stop(): Promise { + while (this._callOnStop.length) { + let methodToCall = this._callOnStop.pop(); + methodToCall(); + } + let cleanupPromise: Promise; if (this._telemetryIntervalId !== undefined) { @@ -467,35 +474,7 @@ export abstract class OmnisharpServer { }); } - protected abstract _makeNextRequest(path: string, data: any): Promise; -} - -export class StdioOmnisharpServer extends OmnisharpServer { - - private static _nextId = 1; - private static StartupTimeout = 1000 * 60; - - private _readLine: ReadLine; - private _activeRequests: { [seq: number]: { onSuccess: Function; onError: Function; } } = Object.create(null); - private _callOnStop: Function[] = []; - - constructor(reporter: TelemetryReporter) { - super(reporter); - - // extra argv - this._extraArgs.push('--stdio'); - } - - public stop(): Promise { - while (this._callOnStop.length) { - let method = this._callOnStop.pop(); - method(); - } - - return super.stop(); - } - - protected _doConnect(): Promise { + private _doConnect(): Promise { this._serverProcess.stderr.on('data', (data: any) => { this._fireEvent('stderr', String(data)); @@ -610,9 +589,9 @@ export class StdioOmnisharpServer extends OmnisharpServer { } } - protected _makeNextRequest(path: string, data: any): Promise { + private _makeNextRequest(path: string, data: any): Promise { - const id = StdioOmnisharpServer._nextId++; + const id = OmnisharpServer._nextId++; const thisRequestPacket: protocol.WireProtocol.RequestPacket = { Type: 'request', From f458f3b6c10870c27c5a39454396aefc8ab76089 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Tue, 8 Nov 2016 15:37:08 -0800 Subject: [PATCH 3/9] Rename OmnisharpServer to OmniSharpServer --- src/assets.ts | 6 ++--- src/features/abstractProvider.ts | 6 ++--- src/features/changeForwarding.ts | 8 +++--- src/features/codeActionProvider.ts | 4 +-- src/features/commands.ts | 12 ++++----- src/features/diagnosticsProvider.ts | 10 ++++---- src/features/dotnetTest.ts | 10 ++++---- src/features/status.ts | 10 ++++---- src/omnisharp/extension.ts | 4 +-- src/omnisharp/server.ts | 4 +-- src/omnisharp/utils.ts | 40 ++++++++++++++--------------- 11 files changed, 57 insertions(+), 57 deletions(-) diff --git a/src/assets.ts b/src/assets.ts index 45fc61f..0e417fa 100644 --- a/src/assets.ts +++ b/src/assets.ts @@ -9,7 +9,7 @@ import * as fs from 'fs-extra-promise'; import * as path from 'path'; import * as vscode from 'vscode'; import * as tasks from 'vscode-tasks'; -import {OmnisharpServer} from './omnisharp/server'; +import {OmniSharpServer} from './omnisharp/server'; import * as serverUtils from './omnisharp/utils'; import * as protocol from './omnisharp/protocol' @@ -393,7 +393,7 @@ export enum AddAssetResult { Cancelled } -export function addAssetsIfNecessary(server: OmnisharpServer): Promise { +export function addAssetsIfNecessary(server: OmniSharpServer): Promise { return new Promise((resolve, reject) => { if (!vscode.workspace.rootPath) { return resolve(AddAssetResult.NotApplicable); @@ -492,7 +492,7 @@ function shouldGenerateAssets(paths: Paths) { }); } -export function generateAssets(server: OmnisharpServer) { +export function generateAssets(server: OmniSharpServer) { serverUtils.requestWorkspaceInformation(server).then(info => { if (info.DotNet && info.DotNet.Projects.length > 0) { getOperations().then(operations => { diff --git a/src/features/abstractProvider.ts b/src/features/abstractProvider.ts index 8a67879..9e7785d 100644 --- a/src/features/abstractProvider.ts +++ b/src/features/abstractProvider.ts @@ -5,15 +5,15 @@ 'use strict'; -import {OmnisharpServer} from '../omnisharp/server'; +import {OmniSharpServer} from '../omnisharp/server'; import {Disposable} from 'vscode'; export default class AbstractProvider { - protected _server: OmnisharpServer; + protected _server: OmniSharpServer; protected _disposables: Disposable[]; - constructor(server: OmnisharpServer) { + constructor(server: OmniSharpServer) { this._server = server; this._disposables = []; } diff --git a/src/features/changeForwarding.ts b/src/features/changeForwarding.ts index c1e555c..ddf586d 100644 --- a/src/features/changeForwarding.ts +++ b/src/features/changeForwarding.ts @@ -6,10 +6,10 @@ 'use strict'; import {Disposable, Uri, workspace} from 'vscode'; -import {OmnisharpServer} from '../omnisharp/server'; +import {OmniSharpServer} from '../omnisharp/server'; import * as serverUtils from '../omnisharp/utils'; -function forwardDocumentChanges(server: OmnisharpServer): Disposable { +function forwardDocumentChanges(server: OmniSharpServer): Disposable { return workspace.onDidChangeTextDocument(event => { @@ -29,7 +29,7 @@ function forwardDocumentChanges(server: OmnisharpServer): Disposable { }); } -function forwardFileChanges(server: OmnisharpServer): Disposable { +function forwardFileChanges(server: OmniSharpServer): Disposable { function onFileSystemEvent(uri: Uri): void { if (!server.isRunning()) { @@ -52,7 +52,7 @@ function forwardFileChanges(server: OmnisharpServer): Disposable { return Disposable.from(watcher, d1, d2, d3); } -export default function forwardChanges(server: OmnisharpServer): Disposable { +export default function forwardChanges(server: OmniSharpServer): Disposable { // combine file watching and text document watching return Disposable.from( diff --git a/src/features/codeActionProvider.ts b/src/features/codeActionProvider.ts index 7254894..d6d97f4 100644 --- a/src/features/codeActionProvider.ts +++ b/src/features/codeActionProvider.ts @@ -6,7 +6,7 @@ 'use strict'; import {CodeActionProvider, CodeActionContext, Command, CancellationToken, TextDocument, WorkspaceEdit, TextEdit, Range, Uri, workspace, commands} from 'vscode'; -import {OmnisharpServer} from '../omnisharp/server'; +import {OmniSharpServer} from '../omnisharp/server'; import AbstractProvider from './abstractProvider'; import * as protocol from '../omnisharp/protocol'; import {toRange2} from '../omnisharp/typeConvertion'; @@ -17,7 +17,7 @@ export default class OmnisharpCodeActionProvider extends AbstractProvider implem private _disabled: boolean; private _commandId: string; - constructor(server: OmnisharpServer) { + constructor(server: OmniSharpServer) { super(server); this._commandId = 'omnisharp.runCodeAction'; diff --git a/src/features/commands.ts b/src/features/commands.ts index 4c787b3..c754b52 100644 --- a/src/features/commands.ts +++ b/src/features/commands.ts @@ -5,7 +5,7 @@ 'use strict'; -import {OmnisharpServer} from '../omnisharp/server'; +import {OmniSharpServer} from '../omnisharp/server'; import * as serverUtils from '../omnisharp/utils'; import {findLaunchTargets} from '../omnisharp/launcher'; import * as cp from 'child_process'; @@ -19,7 +19,7 @@ import {generateAssets} from '../assets'; let channel = vscode.window.createOutputChannel('.NET'); -export default function registerCommands(server: OmnisharpServer, extensionPath: string) { +export default function registerCommands(server: OmniSharpServer, extensionPath: string) { let d1 = vscode.commands.registerCommand('o.restart', () => restartOmniSharp(server)); let d2 = vscode.commands.registerCommand('o.pickProjectAndStart', () => pickProjectAndStart(server)); let d3 = vscode.commands.registerCommand('o.showOutput', () => server.getChannel().show(vscode.ViewColumn.Three)); @@ -44,7 +44,7 @@ export default function registerCommands(server: OmnisharpServer, extensionPath: return vscode.Disposable.from(d1, d2, d3, d4, d5, d6, d7, d8, d9); } -function restartOmniSharp(server: OmnisharpServer) { +function restartOmniSharp(server: OmniSharpServer) { if (server.isRunning()) { server.restart(); } @@ -53,7 +53,7 @@ function restartOmniSharp(server: OmnisharpServer) { } } -function pickProjectAndStart(server: OmnisharpServer) { +function pickProjectAndStart(server: OmniSharpServer) { return findLaunchTargets().then(targets => { @@ -103,7 +103,7 @@ function projectsToCommands(projects: protocol.DotNetProject[]): Promise runDotnetTest(testMethod, fileName, server)); } -export function registerDotNetTestDebugCommand(server: OmnisharpServer): vscode.Disposable { +export function registerDotNetTestDebugCommand(server: OmniSharpServer): vscode.Disposable { return vscode.commands.registerCommand( 'dotnet.test.debug', (testMethod, fileName) => debugDotnetTest(testMethod, fileName, server)); } // Run test through dotnet-test command. This function can be moved to a separate structure -export function runDotnetTest(testMethod: string, fileName: string, server: OmnisharpServer) { +export function runDotnetTest(testMethod: string, fileName: string, server: OmniSharpServer) { getTestOutputChannel().show(); getTestOutputChannel().appendLine('Running test ' + testMethod + '...'); serverUtils @@ -54,7 +54,7 @@ export function runDotnetTest(testMethod: string, fileName: string, server: Omni } // Run test through dotnet-test command with debugger attached -export function debugDotnetTest(testMethod: string, fileName: string, server: OmnisharpServer) { +export function debugDotnetTest(testMethod: string, fileName: string, server: OmniSharpServer) { serverUtils.getTestStartInfo(server, { FileName: fileName, MethodName: testMethod }).then(response => { vscode.commands.executeCommand( 'vscode.startDebug', { diff --git a/src/features/status.ts b/src/features/status.ts index 22ac465..371f5e1 100644 --- a/src/features/status.ts +++ b/src/features/status.ts @@ -5,7 +5,7 @@ 'use strict'; import * as vscode from 'vscode'; -import {OmnisharpServer} from '../omnisharp/server'; +import {OmniSharpServer} from '../omnisharp/server'; import {dotnetRestoreForProject} from './commands'; import {basename} from 'path'; import * as protocol from '../omnisharp/protocol'; @@ -13,7 +13,7 @@ import * as serverUtils from '../omnisharp/utils'; const debounce = require('lodash.debounce'); -export default function reportStatus(server: OmnisharpServer) { +export default function reportStatus(server: OmniSharpServer) { return vscode.Disposable.from( reportServerStatus(server), forwardOutput(server), @@ -41,7 +41,7 @@ class Status { } } -export function reportDocumentStatus(server: OmnisharpServer): vscode.Disposable { +export function reportDocumentStatus(server: OmniSharpServer): vscode.Disposable { let disposables: vscode.Disposable[] = []; let localDisposables: vscode.Disposable[]; @@ -202,7 +202,7 @@ export function reportDocumentStatus(server: OmnisharpServer): vscode.Disposable // ---- server status -export function reportServerStatus(server: OmnisharpServer): vscode.Disposable{ +export function reportServerStatus(server: OmniSharpServer): vscode.Disposable{ function appendLine(value: string = '') { server.getChannel().appendLine(value); @@ -275,7 +275,7 @@ function showMessageSoon() { // --- mirror output in channel -function forwardOutput(server: OmnisharpServer) { +function forwardOutput(server: OmniSharpServer) { const logChannel = server.getChannel(); const timing200Pattern = /^\[INFORMATION:OmniSharp.Middleware.LoggingMiddleware\] \/\w+: 200 \d+ms/; diff --git a/src/omnisharp/extension.ts b/src/omnisharp/extension.ts index 0839da6..ba0957c 100644 --- a/src/omnisharp/extension.ts +++ b/src/omnisharp/extension.ts @@ -23,7 +23,7 @@ import SignatureHelpProvider from '../features/signatureHelpProvider'; import registerCommands from '../features/commands'; import forwardChanges from '../features/changeForwarding'; import reportStatus from '../features/status'; -import {OmnisharpServer} from './server'; +import {OmniSharpServer} from './server'; import {Options} from './options'; import {addAssetsIfNecessary, AddAssetResult} from '../assets'; @@ -33,7 +33,7 @@ export function activate(context: vscode.ExtensionContext, reporter: TelemetryRe scheme: 'file' // only files from disk }; - const server = new OmnisharpServer(reporter); + const server = new OmniSharpServer(reporter); const advisor = new Advisor(server); // create before server is started const disposables: vscode.Disposable[] = []; const localDisposables: vscode.Disposable[] = []; diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index 119ab27..f991d48 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -64,7 +64,7 @@ module Events { const TelemetryReportingDelay = 2 * 60 * 1000; // two minutes -export class OmnisharpServer { +export class OmniSharpServer { private static _nextId = 1; private static StartupTimeout = 1000 * 60; @@ -591,7 +591,7 @@ export class OmnisharpServer { private _makeNextRequest(path: string, data: any): Promise { - const id = OmnisharpServer._nextId++; + const id = OmniSharpServer._nextId++; const thisRequestPacket: protocol.WireProtocol.RequestPacket = { Type: 'request', diff --git a/src/omnisharp/utils.ts b/src/omnisharp/utils.ts index e91e2d2..60641bc 100644 --- a/src/omnisharp/utils.ts +++ b/src/omnisharp/utils.ts @@ -5,82 +5,82 @@ 'use strict'; -import {OmnisharpServer} from './server'; +import {OmniSharpServer} from './server'; import * as protocol from './protocol'; import * as vscode from 'vscode'; -export function autoComplete(server: OmnisharpServer, request: protocol.AutoCompleteRequest) { +export function autoComplete(server: OmniSharpServer, request: protocol.AutoCompleteRequest) { return server.makeRequest(protocol.Requests.AutoComplete, request); } -export function codeCheck(server: OmnisharpServer, request: protocol.Request, token: vscode.CancellationToken) { +export function codeCheck(server: OmniSharpServer, request: protocol.Request, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.CodeCheck, request, token); } -export function currentFileMembersAsTree(server: OmnisharpServer, request: protocol.Request, token: vscode.CancellationToken) { +export function currentFileMembersAsTree(server: OmniSharpServer, request: protocol.Request, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.CurrentFileMembersAsTree, request, token); } -export function filesChanged(server: OmnisharpServer, requests: protocol.Request[]) { +export function filesChanged(server: OmniSharpServer, requests: protocol.Request[]) { return server.makeRequest(protocol.Requests.FilesChanged, requests); } -export function findSymbols(server: OmnisharpServer, request: protocol.FindSymbolsRequest, token: vscode.CancellationToken) { +export function findSymbols(server: OmniSharpServer, request: protocol.FindSymbolsRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.FindSymbols, request, token); } -export function findUsages(server: OmnisharpServer, request: protocol.FindUsagesRequest, token: vscode.CancellationToken) { +export function findUsages(server: OmniSharpServer, request: protocol.FindUsagesRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.FindUsages, request, token); } -export function formatAfterKeystroke(server: OmnisharpServer, request: protocol.FormatAfterKeystrokeRequest, token: vscode.CancellationToken) { +export function formatAfterKeystroke(server: OmniSharpServer, request: protocol.FormatAfterKeystrokeRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.FormatAfterKeystroke, request, token); } -export function formatRange(server: OmnisharpServer, request: protocol.FormatRangeRequest, token: vscode.CancellationToken) { +export function formatRange(server: OmniSharpServer, request: protocol.FormatRangeRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.FormatRange, request, token); } -export function getCodeActions(server: OmnisharpServer, request: protocol.V2.GetCodeActionsRequest, token: vscode.CancellationToken) { +export function getCodeActions(server: OmniSharpServer, request: protocol.V2.GetCodeActionsRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.V2.Requests.GetCodeActions, request, token); } -export function goToDefinition(server: OmnisharpServer, request: protocol.GoToDefinitionRequest, token: vscode.CancellationToken) { +export function goToDefinition(server: OmniSharpServer, request: protocol.GoToDefinitionRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.GoToDefinition, request); } -export function rename(server: OmnisharpServer, request: protocol.RenameRequest, token: vscode.CancellationToken) { +export function rename(server: OmniSharpServer, request: protocol.RenameRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.Rename, request, token); } -export function requestWorkspaceInformation(server: OmnisharpServer) { +export function requestWorkspaceInformation(server: OmniSharpServer) { return server.makeRequest(protocol.Requests.Projects); } -export function runCodeAction(server: OmnisharpServer, request: protocol.V2.RunCodeActionRequest) { +export function runCodeAction(server: OmniSharpServer, request: protocol.V2.RunCodeActionRequest) { return server.makeRequest(protocol.V2.Requests.RunCodeAction, request); } -export function signatureHelp(server: OmnisharpServer, request: protocol.Request, token: vscode.CancellationToken) { +export function signatureHelp(server: OmniSharpServer, request: protocol.Request, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.SignatureHelp, request, token); } -export function typeLookup(server: OmnisharpServer, request: protocol.TypeLookupRequest, token: vscode.CancellationToken) { +export function typeLookup(server: OmniSharpServer, request: protocol.TypeLookupRequest, token: vscode.CancellationToken) { return server.makeRequest(protocol.Requests.TypeLookup, request, token); } -export function updateBuffer(server: OmnisharpServer, request: protocol.UpdateBufferRequest) { +export function updateBuffer(server: OmniSharpServer, request: protocol.UpdateBufferRequest) { return server.makeRequest(protocol.Requests.UpdateBuffer, request); } -export function getMetadata(server: OmnisharpServer, request: protocol.MetadataRequest) { +export function getMetadata(server: OmniSharpServer, request: protocol.MetadataRequest) { return server.makeRequest(protocol.Requests.Metadata, request); } -export function getTestStartInfo(server: OmnisharpServer, request: protocol.V2.GetTestStartInfoRequest) { +export function getTestStartInfo(server: OmniSharpServer, request: protocol.V2.GetTestStartInfoRequest) { return server.makeRequest(protocol.V2.Requests.GetTestStartInfo, request); } -export function runDotNetTest(server: OmnisharpServer, request: protocol.V2.RunDotNetTestRequest) { +export function runDotNetTest(server: OmniSharpServer, request: protocol.V2.RunDotNetTestRequest) { return server.makeRequest(protocol.V2.Requests.RunDotNetTest, request); } \ No newline at end of file From 4f39877a413adf33701fa923e6aab985386f81de Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Wed, 9 Nov 2016 11:43:53 -0800 Subject: [PATCH 4/9] Moar server work --- src/omnisharp/queue.ts | 84 ++++++++++++----- src/omnisharp/server.ts | 196 +++++++++++++++------------------------- 2 files changed, 135 insertions(+), 145 deletions(-) diff --git a/src/omnisharp/queue.ts b/src/omnisharp/queue.ts index 7608d14..426cda2 100644 --- a/src/omnisharp/queue.ts +++ b/src/omnisharp/queue.ts @@ -8,58 +8,89 @@ import * as protocol from './protocol'; import * as prioritization from './prioritization'; interface Request { - path: string; + command: string; data?: any; onSuccess(value: any): void; onError(err: any): void; } +/** + * This data structure manages a queue of requests that have been made and requests that have been + * sent to the OmniSharp server and are waiting on a response. + */ class RequestQueue { private _pending: Request[] = []; - private _requests: Map = new Map(); + private _waiting: Map = new Map(); public constructor( + private _name: string, private _maxSize: number, private _logger: Logger, private _makeRequest: (request: Request) => protocol.WireProtocol.RequestPacket) { } + /** + * Enqueue a new request. + */ public enqueue(request: Request) { + this._logger.appendLine(`Enqueuing request for ${request.command}.`); this._pending.push(request); } - public remove(command: string, seq: number) { - if (!this._requests.delete(seq)) { - this._logger.appendLine(`Received response for ${command} but could not find request.`); + public dequeue(seq: number) { + return this._waiting.delete(seq); + } + + public delete(request: Request) { + let index = this._pending.indexOf(request); + if (index !== -1) { + this._pending.splice(index, 1); + + // Do something better here. + let err = new Error('Canceled'); + err.message = 'Canceled'; + request.onError(err); } } + /** + * Returns true if there are any requests pending to be sent to the OmniSharp server. + */ public hasPending() { - return this._pending.length === 0; + return this._pending.length > 0; } + /** + * Returns true if the maximum number of requests waiting on the OmniSharp server has been reached. + */ public isFull() { - return this._requests.size >= this._maxSize; + return this._waiting.size >= this._maxSize; } - public drain() { - let i = 0; - const slots = this._maxSize - this._requests.size; + /** + * Process any pending requests and send them to the OmniSharp server. + */ + public processPending() { + if (this._pending.length === 0) { + return; + } - do { + const slots = this._maxSize - this._waiting.size; + + for (let i = 0; i < slots && this._pending.length > 0; i++) { const item = this._pending.shift(); const request = this._makeRequest(item); - this._requests.set(request.Seq, request); + this._waiting.set(request.Seq, request); if (this.isFull()) { return; } } - while (this._pending.length > 0 && ++i < slots) } } export class Queue { + private _logger: Logger; private _isProcessing: boolean; private _priorityQueue: RequestQueue; private _normalQueue: RequestQueue; @@ -70,9 +101,9 @@ export class Queue { concurrency: number, makeRequest: (request: Request) => protocol.WireProtocol.RequestPacket ) { - this._priorityQueue = new RequestQueue(1, logger, makeRequest); - this._normalQueue = new RequestQueue(concurrency, logger, makeRequest); - this._deferredQueue = new RequestQueue(Math.max(Math.floor(concurrency / 4), 2), logger, makeRequest); + this._priorityQueue = new RequestQueue('Priority', 1, logger, makeRequest); + this._normalQueue = new RequestQueue('Normal', concurrency, logger, makeRequest); + this._deferredQueue = new RequestQueue('Deferred', Math.max(Math.floor(concurrency / 4), 2), logger, makeRequest); } private getQueue(command: string) { @@ -88,18 +119,23 @@ export class Queue { } public enqueue(request: Request) { - const queue = this.getQueue(request.path); + const queue = this.getQueue(request.command); queue.enqueue(request); this.drain(); } - public remove(command: string, seq: number) { + public dequeue(command: string, seq: number) { const queue = this.getQueue(command); - queue.remove(command, seq); + return queue.dequeue(seq); } - private drain() { + public cancelRequest(request: Request) { + const queue = this.getQueue(request.command); + queue.delete(request); + } + + public drain() { if (this._isProcessing) { return false; } @@ -112,18 +148,20 @@ export class Queue { return false; } + this._isProcessing = true; + if (this._priorityQueue.hasPending()) { - this._priorityQueue.drain(); + this._priorityQueue.processPending(); this._isProcessing = false; return; } if (this._normalQueue.hasPending()) { - this._normalQueue.drain(); + this._normalQueue.processPending(); } if (this._deferredQueue.hasPending()) { - this._deferredQueue.drain(); + this._deferredQueue.processPending(); } this._isProcessing = false; diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index f991d48..d02714d 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -16,6 +16,7 @@ import {Logger} from '../logger'; import {DelayTracker} from './delayTracker'; import {LaunchTarget, findLaunchTargets} from './launcher'; import {PlatformInformation} from '../platform'; +import { Queue } from './queue'; import TelemetryReporter from 'vscode-extension-telemetry'; import * as vscode from 'vscode'; @@ -26,11 +27,10 @@ enum ServerState { } interface Request { - path: string; + command: string; data?: any; onSuccess(value: any): void; onError(err: any): void; - _enqueued: number; } module Events { @@ -70,8 +70,7 @@ export class OmniSharpServer { private static StartupTimeout = 1000 * 60; private _readLine: ReadLine; - private _activeRequests: { [seq: number]: { onSuccess: Function; onError: Function; } } = Object.create(null); - private _callOnStop: Function[] = []; + private _disposables: vscode.Disposable[] = []; private _reporter: TelemetryReporter; private _delayTrackers: { [requestName: string]: DelayTracker }; @@ -80,8 +79,7 @@ export class OmniSharpServer { private _eventBus = new EventEmitter(); private _state: ServerState = ServerState.Stopped; private _launchTarget: LaunchTarget; - private _queue: Request[] = []; - private _isProcessingQueue = false; + private _requestQueue: Queue; private _channel: vscode.OutputChannel; private _logger: Logger; @@ -95,6 +93,7 @@ export class OmniSharpServer { this._channel = vscode.window.createOutputChannel('OmniSharp Log'); this._logger = new Logger(message => this._channel.append(message)); + this._requestQueue = new Queue(this._logger, 8, request => this._makeRequest(request)); } public isRunning(): boolean { @@ -292,7 +291,7 @@ export class OmniSharpServer { // Start telemetry reporting this._telemetryIntervalId = setInterval(() => this._reportTelemetry(), TelemetryReportingDelay); }).then(() => { - this._processQueue(); + this._requestQueue.drain(); }).catch(err => { this._fireEvent(Events.ServerError, err); return this.stop(); @@ -301,9 +300,8 @@ export class OmniSharpServer { public stop(): Promise { - while (this._callOnStop.length) { - let methodToCall = this._callOnStop.pop(); - methodToCall(); + while (this._disposables.length) { + this._disposables.pop().dispose(); } let cleanupPromise: Promise; @@ -400,13 +398,13 @@ export class OmniSharpServer { // --- requests et al - public makeRequest(path: string, data?: any, token?: vscode.CancellationToken): Promise { + public makeRequest(command: string, data?: any, token?: vscode.CancellationToken): Promise { if (this._getState() !== ServerState.Started) { return Promise.reject('server has been stopped or not started'); } - console.log(`makeRequest: path=${path}`); + console.log(`makeRequest: command=${command}`); let startTime: number; let request: Request; @@ -415,65 +413,34 @@ export class OmniSharpServer { startTime = Date.now(); request = { - path, + command, data, onSuccess: value => resolve(value), - onError: err => reject(err), - _enqueued: Date.now() + onError: err => reject(err) }; - this._queue.push(request); + this._requestQueue.enqueue(request); - if (this._getState() === ServerState.Started && !this._isProcessingQueue) { - this._processQueue(); + if (this._getState() === ServerState.Started) { + this._requestQueue.drain(); } }); if (token) { token.onCancellationRequested(() => { - let idx = this._queue.indexOf(request); - if (idx !== -1) { - this._queue.splice(idx, 1); - let err = new Error('Canceled'); - err.message = 'Canceled'; - request.onError(err); - } + this._requestQueue.cancelRequest(request); }); } return promise.then(response => { let endTime = Date.now(); let elapsedTime = endTime - startTime; - this._recordRequestDelay(path, elapsedTime); + this._recordRequestDelay(command, elapsedTime); return response; }); } - private _processQueue(): void { - if (this._queue.length === 0) { - // nothing to do - this._isProcessingQueue = false; - return; - } - - // signal that we are working on it - this._isProcessingQueue = true; - - // send next request and recurse when done - const thisRequest = this._queue.shift(); - this._makeNextRequest(thisRequest.path, thisRequest.data).then(value => { - thisRequest.onSuccess(value); - this._processQueue(); - }, err => { - thisRequest.onError(err); - this._processQueue(); - }).catch(err => { - console.error(err); - this._processQueue(); - }); - } - private _doConnect(): Promise { this._serverProcess.stderr.on('data', (data: any) => { @@ -486,11 +453,11 @@ export class OmniSharpServer { terminal: false }); - const p = new Promise((resolve, reject) => { + const promise = new Promise((resolve, reject) => { let listener: vscode.Disposable; // Convert the timeout from the seconds to milliseconds, which is required by setTimeout(). - const timeoutDuration = this._options.projectLoadTimeout * 1000 + const timeoutDuration = this._options.projectLoadTimeout * 10000 // timeout logic const handle = setTimeout(() => { @@ -511,105 +478,90 @@ export class OmniSharpServer { }); }); - this._startListening(); + const lineReceived = this._onLineReceived.bind(this); - return p; + this._readLine.addListener('line', lineReceived); + + this._disposables.push(new vscode.Disposable(() => { + this._readLine.removeListener('line', lineReceived); + })); + + return promise; } - private _startListening(): void { - - const onLineReceived = (line: string) => { - if (line[0] !== '{') { - this._logger.appendLine(line); - return; - } - - let packet: protocol.WireProtocol.Packet; - try { - packet = JSON.parse(line); - } - catch (e) { - // not json - return; - } - - if (!packet.Type) { - // bogous packet - return; - } - - switch (packet.Type) { - case 'response': - this._handleResponsePacket(packet); - break; - case 'event': - this._handleEventPacket(packet); - break; - default: - console.warn('unknown packet: ', packet); - break; - } - }; - - this._readLine.addListener('line', onLineReceived); - this._callOnStop.push(() => this._readLine.removeListener('line', onLineReceived)); - } - - private _handleResponsePacket(packet: protocol.WireProtocol.ResponsePacket): void { - - const id = packet.Request_seq; - const entry = this._activeRequests[id]; - - if (!entry) { - console.warn('Received a response WITHOUT a request', packet); + private _onLineReceived(line: string) { + if (line[0] !== '{') { + this._logger.appendLine(line); return; } - console.log(`Handling response: ${packet.Command} (${id})`); + let packet: protocol.WireProtocol.Packet; + try { + packet = JSON.parse(line); + } + catch (err) { + // This isn't JSON + return; + } - delete this._activeRequests[id]; + if (!packet.Type) { + // Bogus packet + return; + } + + switch (packet.Type) { + case 'response': + this._handleResponsePacket(packet); + break; + case 'event': + this._handleEventPacket(packet); + break; + default: + console.warn(`Unknown packet type: ${packet.Type}`); + break; + } + } + + private _handleResponsePacket(packet: protocol.WireProtocol.ResponsePacket) { + if (!this._requestQueue.dequeue(packet.Command, packet.Request_seq)) { + this._logger.appendLine(`Received response for ${packet.Command} but could not find request.`); + return; + } if (packet.Success) { - entry.onSuccess(packet.Body); - } else { - entry.onError(packet.Message || packet.Body); + // Handle success + } + else { + // Handle failure } } private _handleEventPacket(packet: protocol.WireProtocol.EventPacket): void { - if (packet.Event === 'log') { // handle log events const entry = <{ LogLevel: string; Name: string; Message: string; }>packet.Body; this._logger.appendLine(`[${entry.LogLevel}:${entry.Name}] ${entry.Message}`); - return; - } else { + } + else { // fwd all other events this._fireEvent(packet.Event, packet.Body); } } - private _makeNextRequest(path: string, data: any): Promise { - + private _makeRequest(request: Request) { const id = OmniSharpServer._nextId++; - const thisRequestPacket: protocol.WireProtocol.RequestPacket = { + const requestPacket: protocol.WireProtocol.RequestPacket = { Type: 'request', Seq: id, - Command: path, - Arguments: data + Command: request.command, + Arguments: request.data }; - console.log(`Making request: ${path} (${id})`); + console.log(`Making request: ${request.command} (${id})`); - return new Promise((resolve, reject) => { + this._serverProcess.stdin.write(JSON.stringify(requestPacket) + '\n'); - this._activeRequests[thisRequestPacket.Seq] = { - onSuccess: value => resolve(value), - onError: err => reject(err) - }; - - this._serverProcess.stdin.write(JSON.stringify(thisRequestPacket) + '\n'); - }); + return requestPacket; } } From c9db693cc6838fce09d9a0ea70e2ecb58f70ad7a Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Wed, 9 Nov 2016 12:52:59 -0800 Subject: [PATCH 5/9] Get server working again --- src/features/status.ts | 10 +-- src/omnisharp/{queue.ts => requestQueue.ts} | 46 ++++++++----- src/omnisharp/server.ts | 72 +++++++++++---------- 3 files changed, 70 insertions(+), 58 deletions(-) rename src/omnisharp/{queue.ts => requestQueue.ts} (77%) diff --git a/src/features/status.ts b/src/features/status.ts index 371f5e1..19086fe 100644 --- a/src/features/status.ts +++ b/src/features/status.ts @@ -278,17 +278,11 @@ function showMessageSoon() { function forwardOutput(server: OmniSharpServer) { const logChannel = server.getChannel(); - const timing200Pattern = /^\[INFORMATION:OmniSharp.Middleware.LoggingMiddleware\] \/\w+: 200 \d+ms/; function forward(message: string) { - // strip stuff like: /codecheck: 200 339ms - if(!timing200Pattern.test(message)) { - logChannel.append(message); - } + logChannel.append(message); } return vscode.Disposable.from( - server.onStdout(forward), server.onStderr(forward)); -} - +} \ No newline at end of file diff --git a/src/omnisharp/queue.ts b/src/omnisharp/requestQueue.ts similarity index 77% rename from src/omnisharp/queue.ts rename to src/omnisharp/requestQueue.ts index 426cda2..2fb0b69 100644 --- a/src/omnisharp/queue.ts +++ b/src/omnisharp/requestQueue.ts @@ -7,11 +7,13 @@ import { Logger } from '../logger'; import * as protocol from './protocol'; import * as prioritization from './prioritization'; -interface Request { +export interface Request { command: string; data?: any; onSuccess(value: any): void; onError(err: any): void; + startTime?: number; + endTime?: number; } /** @@ -20,37 +22,47 @@ interface Request { */ class RequestQueue { private _pending: Request[] = []; - private _waiting: Map = new Map(); + private _waiting: Map = new Map(); public constructor( private _name: string, private _maxSize: number, private _logger: Logger, - private _makeRequest: (request: Request) => protocol.WireProtocol.RequestPacket) { + private _makeRequest: (request: Request) => number) { } /** * Enqueue a new request. */ public enqueue(request: Request) { - this._logger.appendLine(`Enqueuing request for ${request.command}.`); + this._logger.appendLine(`Enqueue request for ${request.command}.`); this._pending.push(request); } - public dequeue(seq: number) { - return this._waiting.delete(seq); + /** + * Dequeue a request that has completed. + */ + public dequeue(id: number) { + const request = this._waiting.get(id); + + if (request) { + this._waiting.delete(id); + this._logger.appendLine(`Dequeue request for ${request.command}.`); + } + + return request; } - public delete(request: Request) { + public cancelRequest(request: Request) { let index = this._pending.indexOf(request); if (index !== -1) { this._pending.splice(index, 1); - // Do something better here. - let err = new Error('Canceled'); - err.message = 'Canceled'; - request.onError(err); + // Note: This calls reject() on the promise returned by OmniSharpServer.makeRequest + request.onError(new Error(`Pending request cancelled: ${request.command}`)); } + + // TODO: Handle cancellation of a request already waiting on the OmniSharp server. } /** @@ -79,8 +91,10 @@ class RequestQueue { for (let i = 0; i < slots && this._pending.length > 0; i++) { const item = this._pending.shift(); - const request = this._makeRequest(item); - this._waiting.set(request.Seq, request); + item.startTime = Date.now(); + + const id = this._makeRequest(item); + this._waiting.set(id, item); if (this.isFull()) { return; @@ -89,7 +103,7 @@ class RequestQueue { } } -export class Queue { +export class RequestQueueCollection { private _logger: Logger; private _isProcessing: boolean; private _priorityQueue: RequestQueue; @@ -99,7 +113,7 @@ export class Queue { public constructor( logger: Logger, concurrency: number, - makeRequest: (request: Request) => protocol.WireProtocol.RequestPacket + makeRequest: (request: Request) => number ) { this._priorityQueue = new RequestQueue('Priority', 1, logger, makeRequest); this._normalQueue = new RequestQueue('Normal', concurrency, logger, makeRequest); @@ -132,7 +146,7 @@ export class Queue { public cancelRequest(request: Request) { const queue = this.getQueue(request.command); - queue.delete(request); + queue.cancelRequest(request); } public drain() { diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index d02714d..696a2da 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -3,21 +3,19 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -'use strict'; - -import {EventEmitter} from 'events'; -import {ChildProcess, exec} from 'child_process'; -import {dirname} from 'path'; -import {ReadLine, createInterface} from 'readline'; -import {launchOmniSharp} from './launcher'; -import * as protocol from './protocol'; -import {Options} from './options'; -import {Logger} from '../logger'; -import {DelayTracker} from './delayTracker'; -import {LaunchTarget, findLaunchTargets} from './launcher'; -import {PlatformInformation} from '../platform'; -import { Queue } from './queue'; +import { EventEmitter } from 'events'; +import { ChildProcess, exec } from 'child_process'; +import { ReadLine, createInterface } from 'readline'; +import { launchOmniSharp } from './launcher'; +import { Options } from './options'; +import { Logger } from '../logger'; +import { DelayTracker } from './delayTracker'; +import { LaunchTarget, findLaunchTargets } from './launcher'; +import { PlatformInformation } from '../platform'; +import { Request, RequestQueueCollection } from './requestQueue'; import TelemetryReporter from 'vscode-extension-telemetry'; +import * as path from 'path'; +import * as protocol from './protocol'; import * as vscode from 'vscode'; enum ServerState { @@ -26,13 +24,6 @@ enum ServerState { Stopped } -interface Request { - command: string; - data?: any; - onSuccess(value: any): void; - onError(err: any): void; -} - module Events { export const StateChanged = 'stateChanged'; @@ -79,7 +70,7 @@ export class OmniSharpServer { private _eventBus = new EventEmitter(); private _state: ServerState = ServerState.Stopped; private _launchTarget: LaunchTarget; - private _requestQueue: Queue; + private _requestQueue: RequestQueueCollection; private _channel: vscode.OutputChannel; private _logger: Logger; @@ -93,7 +84,7 @@ export class OmniSharpServer { this._channel = vscode.window.createOutputChannel('OmniSharp Log'); this._logger = new Logger(message => this._channel.append(message)); - this._requestQueue = new Queue(this._logger, 8, request => this._makeRequest(request)); + this._requestQueue = new RequestQueueCollection(this._logger, 8, request => this._makeRequest(request)); } public isRunning(): boolean { @@ -124,9 +115,9 @@ export class OmniSharpServer { private _reportTelemetry() { const delayTrackers = this._delayTrackers; - for (const path in delayTrackers) { - const tracker = delayTrackers[path]; - const eventName = 'omnisharp' + path; + for (const requestName in delayTrackers) { + const tracker = delayTrackers[requestName]; + const eventName = 'omnisharp' + requestName; if (tracker.hasMeasures()) { const measures = tracker.getMeasures(); tracker.clearMeasures(); @@ -237,7 +228,7 @@ export class OmniSharpServer { this._launchTarget = launchTarget; const solutionPath = launchTarget.target; - const cwd = dirname(solutionPath); + const cwd = path.dirname(solutionPath); let args = [ '-s', solutionPath, '--hostPID', process.pid.toString(), @@ -473,6 +464,7 @@ export class OmniSharpServer { if (listener) { listener.dispose(); } + clearTimeout(handle); resolve(); }); @@ -523,24 +515,25 @@ export class OmniSharpServer { } private _handleResponsePacket(packet: protocol.WireProtocol.ResponsePacket) { - if (!this._requestQueue.dequeue(packet.Command, packet.Request_seq)) { + const request = this._requestQueue.dequeue(packet.Command, packet.Request_seq); + + if (!request) { this._logger.appendLine(`Received response for ${packet.Command} but could not find request.`); return; } if (packet.Success) { - // Handle success + request.onSuccess(packet.Body); } else { - // Handle failure + request.onError(packet.Message || packet.Body); } } private _handleEventPacket(packet: protocol.WireProtocol.EventPacket): void { if (packet.Event === 'log') { - // handle log events const entry = <{ LogLevel: string; Name: string; Message: string; }>packet.Body; - this._logger.appendLine(`[${entry.LogLevel}:${entry.Name}] ${entry.Message}`); + this._logOutput(entry.LogLevel, entry.Name, entry.Message); } else { // fwd all other events @@ -558,10 +551,21 @@ export class OmniSharpServer { Arguments: request.data }; - console.log(`Making request: ${request.command} (${id})`); + this._logger.appendLine(`Making request: ${request.command} (${id})`); this._serverProcess.stdin.write(JSON.stringify(requestPacket) + '\n'); - return requestPacket; + return id; + } + + private _logOutput(logLevel: string, name: string, message: string) { + const timing200Pattern = /^\[INFORMATION:OmniSharp.Middleware.LoggingMiddleware\] \/[\/\w]+: 200 \d+ms/; + + const output = `[${logLevel}:${name}] ${message}`; + + // strip stuff like: /codecheck: 200 339ms + if (!timing200Pattern.test(output)) { + this._logger.appendLine(output); + } } } From 82bc2a45cea0a0caf17182decf12e679d44ccb81 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Wed, 9 Nov 2016 13:13:18 -0800 Subject: [PATCH 6/9] Add a bit more logging --- src/omnisharp/requestQueue.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/omnisharp/requestQueue.ts b/src/omnisharp/requestQueue.ts index 2fb0b69..b3f6126 100644 --- a/src/omnisharp/requestQueue.ts +++ b/src/omnisharp/requestQueue.ts @@ -35,7 +35,7 @@ class RequestQueue { * Enqueue a new request. */ public enqueue(request: Request) { - this._logger.appendLine(`Enqueue request for ${request.command}.`); + this._logger.appendLine(`Enqueue ${this._name} request for ${request.command}.`); this._pending.push(request); } @@ -47,7 +47,7 @@ class RequestQueue { if (request) { this._waiting.delete(id); - this._logger.appendLine(`Dequeue request for ${request.command}.`); + this._logger.appendLine(`Dequeue ${this._name} request for ${request.command} (${id}).`); } return request; @@ -87,6 +87,9 @@ class RequestQueue { return; } + this._logger.appendLine(`Processing ${this._name} queue`); + this._logger.increaseIndent(); + const slots = this._maxSize - this._waiting.size; for (let i = 0; i < slots && this._pending.length > 0; i++) { @@ -97,9 +100,11 @@ class RequestQueue { this._waiting.set(id, item); if (this.isFull()) { - return; + break; } } + + this._logger.decreaseIndent(); } } From 57ffb02b4a7ce80a000d69d9eef100ab5aca2f9b Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Wed, 9 Nov 2016 13:45:15 -0800 Subject: [PATCH 7/9] Move extra server logging to 'debug mode' --- src/omnisharp/server.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index 696a2da..6383794 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -60,6 +60,8 @@ export class OmniSharpServer { private static _nextId = 1; private static StartupTimeout = 1000 * 60; + private _debugMode: boolean = false; + private _readLine: ReadLine; private _disposables: vscode.Disposable[] = []; @@ -84,7 +86,12 @@ export class OmniSharpServer { this._channel = vscode.window.createOutputChannel('OmniSharp Log'); this._logger = new Logger(message => this._channel.append(message)); - this._requestQueue = new RequestQueueCollection(this._logger, 8, request => this._makeRequest(request)); + + const logger = this._debugMode + ? this._logger + : new Logger(message => { }); + + this._requestQueue = new RequestQueueCollection(logger, 8, request => this._makeRequest(request)); } public isRunning(): boolean { @@ -448,7 +455,7 @@ export class OmniSharpServer { let listener: vscode.Disposable; // Convert the timeout from the seconds to milliseconds, which is required by setTimeout(). - const timeoutDuration = this._options.projectLoadTimeout * 10000 + const timeoutDuration = this._options.projectLoadTimeout * 1000 // timeout logic const handle = setTimeout(() => { @@ -551,7 +558,9 @@ export class OmniSharpServer { Arguments: request.data }; - this._logger.appendLine(`Making request: ${request.command} (${id})`); + if (this._debugMode) { + this._logger.appendLine(`Making request: ${request.command} (${id})`); + } this._serverProcess.stdin.write(JSON.stringify(requestPacket) + '\n'); @@ -564,7 +573,7 @@ export class OmniSharpServer { const output = `[${logLevel}:${name}] ${message}`; // strip stuff like: /codecheck: 200 339ms - if (!timing200Pattern.test(output)) { + if (this._debugMode || !timing200Pattern.test(output)) { this._logger.appendLine(output); } } From 11d4d7ca302871ca6b8dd0af6dfd6170aa126644 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 10 Nov 2016 00:17:49 -0800 Subject: [PATCH 8/9] Fix a few issues reported by tslint --- src/omnisharp/launcher.ts | 6 +++--- src/omnisharp/requestQueue.ts | 2 -- src/omnisharp/server.ts | 4 +--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/omnisharp/launcher.ts b/src/omnisharp/launcher.ts index a082a17..98b500d 100644 --- a/src/omnisharp/launcher.ts +++ b/src/omnisharp/launcher.ts @@ -144,7 +144,7 @@ export function launchOmniSharp(cwd: string, args: string[], kind: LaunchTargetK .then(result => { // async error - when target not not ENEOT result.process.on('error', err => { - reject(err) + reject(err); }); // success after a short freeing event loop @@ -243,9 +243,9 @@ function launchNixMono(launchPath: string, cwd: string, args: string[]): Promise return canLaunchMono() .then(() => { let argsCopy = args.slice(0); // create copy of details args - args.unshift(launchPath); + argsCopy.unshift(launchPath); - let process = spawn('mono', args, { + let process = spawn('mono', argsCopy, { detached: false, cwd: cwd }); diff --git a/src/omnisharp/requestQueue.ts b/src/omnisharp/requestQueue.ts index b3f6126..26fe650 100644 --- a/src/omnisharp/requestQueue.ts +++ b/src/omnisharp/requestQueue.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import { Logger } from '../logger'; -import * as protocol from './protocol'; import * as prioritization from './prioritization'; export interface Request { @@ -109,7 +108,6 @@ class RequestQueue { } export class RequestQueueCollection { - private _logger: Logger; private _isProcessing: boolean; private _priorityQueue: RequestQueue; private _normalQueue: RequestQueue; diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index 6383794..07718a2 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -11,7 +11,6 @@ import { Options } from './options'; import { Logger } from '../logger'; import { DelayTracker } from './delayTracker'; import { LaunchTarget, findLaunchTargets } from './launcher'; -import { PlatformInformation } from '../platform'; import { Request, RequestQueueCollection } from './requestQueue'; import TelemetryReporter from 'vscode-extension-telemetry'; import * as path from 'path'; @@ -58,7 +57,6 @@ const TelemetryReportingDelay = 2 * 60 * 1000; // two minutes export class OmniSharpServer { private static _nextId = 1; - private static StartupTimeout = 1000 * 60; private _debugMode: boolean = false; @@ -455,7 +453,7 @@ export class OmniSharpServer { let listener: vscode.Disposable; // Convert the timeout from the seconds to milliseconds, which is required by setTimeout(). - const timeoutDuration = this._options.projectLoadTimeout * 1000 + const timeoutDuration = this._options.projectLoadTimeout * 1000; // timeout logic const handle = setTimeout(() => { From 07d572d5e86e03ee4fec3eb9887ac7e016c6ff00 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 10 Nov 2016 00:26:06 -0800 Subject: [PATCH 9/9] Make a few tweaks based on code review feedback --- src/omnisharp/server.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index 07718a2..b900a99 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -416,10 +416,6 @@ export class OmniSharpServer { }; this._requestQueue.enqueue(request); - - if (this._getState() === ServerState.Started) { - this._requestQueue.drain(); - } }); if (token) { @@ -533,6 +529,8 @@ export class OmniSharpServer { else { request.onError(packet.Message || packet.Body); } + + this._requestQueue.drain(); } private _handleEventPacket(packet: protocol.WireProtocol.EventPacket): void {