From 22d3a337fcd3faab6e2d8e63630a01f0255e7919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Spo=CC=88nemann?= Date: Thu, 26 Sep 2024 10:31:25 +0200 Subject: [PATCH 1/2] Improvements for cancelable operations - Use performance.now() for time measurements - Use the startCancelableOperation function to reset the lastTick variable --- packages/langium/src/utils/promise-utils.ts | 4 ++-- packages/langium/src/workspace/workspace-lock.ts | 8 ++++---- .../test/parser/worker-thread-async-parser.test.ts | 12 ++++++------ .../test/workspace/document-builder.test.ts | 14 +++++++------- .../langium/test/workspace/workspace-lock.test.ts | 4 ++-- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/langium/src/utils/promise-utils.ts b/packages/langium/src/utils/promise-utils.ts index 9b730e3e1..c7b8aea99 100644 --- a/packages/langium/src/utils/promise-utils.ts +++ b/packages/langium/src/utils/promise-utils.ts @@ -31,7 +31,7 @@ let globalInterruptionPeriod = 10; * Reset the global interruption period and create a cancellation token source. */ export function startCancelableOperation(): AbstractCancellationTokenSource { - lastTick = Date.now(); + lastTick = performance.now(); return new CancellationTokenSource(); } @@ -74,7 +74,7 @@ export async function interruptAndCheck(token: CancellationToken): Promise // Early exit in case cancellation was disabled by the caller return; } - const current = Date.now(); + const current = performance.now(); if (current - lastTick >= globalInterruptionPeriod) { lastTick = current; await delayNextTick(); diff --git a/packages/langium/src/workspace/workspace-lock.ts b/packages/langium/src/workspace/workspace-lock.ts index 846d9722e..423876d15 100644 --- a/packages/langium/src/workspace/workspace-lock.ts +++ b/packages/langium/src/workspace/workspace-lock.ts @@ -4,8 +4,8 @@ * terms of the MIT License, which is available in the project root. ******************************************************************************/ -import { CancellationToken, CancellationTokenSource } from '../utils/cancellation.js'; -import { Deferred, isOperationCancelled, type MaybePromise } from '../utils/promise-utils.js'; +import { type AbstractCancellationTokenSource, CancellationToken, CancellationTokenSource } from '../utils/cancellation.js'; +import { Deferred, isOperationCancelled, startCancelableOperation, type MaybePromise } from '../utils/promise-utils.js'; /** * Utility service to execute mutually exclusive actions. @@ -47,14 +47,14 @@ interface LockEntry { export class DefaultWorkspaceLock implements WorkspaceLock { - private previousTokenSource = new CancellationTokenSource(); + private previousTokenSource: AbstractCancellationTokenSource = new CancellationTokenSource(); private writeQueue: LockEntry[] = []; private readQueue: LockEntry[] = []; private done = true; write(action: (token: CancellationToken) => MaybePromise): Promise { this.cancelWrite(); - const tokenSource = new CancellationTokenSource(); + const tokenSource = startCancelableOperation(); this.previousTokenSource = tokenSource; return this.enqueue(this.writeQueue, action, tokenSource.token); } diff --git a/packages/langium/test/parser/worker-thread-async-parser.test.ts b/packages/langium/test/parser/worker-thread-async-parser.test.ts index 1dabba00e..96f396832 100644 --- a/packages/langium/test/parser/worker-thread-async-parser.test.ts +++ b/packages/langium/test/parser/worker-thread-async-parser.test.ts @@ -9,8 +9,8 @@ import { WorkerThreadAsyncParser } from 'langium/node'; import { createLangiumGrammarServices } from 'langium/grammar'; import type { Grammar, LangiumCoreServices, ParseResult } from 'langium'; import type { LangiumServices } from 'langium/lsp'; -import { EmptyFileSystem, GrammarUtils, CstUtils, GrammarAST, isOperationCancelled } from 'langium'; -import { CancellationToken, CancellationTokenSource } from 'vscode-languageserver'; +import { EmptyFileSystem, GrammarUtils, CstUtils, GrammarAST, isOperationCancelled, startCancelableOperation } from 'langium'; +import { CancellationToken } from 'vscode-languageserver'; import { fail } from 'node:assert'; import { fileURLToPath } from 'node:url'; @@ -46,16 +46,16 @@ describe('WorkerThreadAsyncParser', () => { // This file should take a few seconds to parse const file = createLargeFile(100000); const asyncParser = services.parser.AsyncParser; - const cancellationTokenSource = new CancellationTokenSource(); + const cancellationTokenSource = startCancelableOperation(); setTimeout(() => cancellationTokenSource.cancel(), 50); - const start = Date.now(); + const start = performance.now(); try { await asyncParser.parse(file, cancellationTokenSource.token); fail('Parsing should have been cancelled'); } catch (err) { expect(isOperationCancelled(err)).toBe(true); } - const end = Date.now(); + const end = performance.now(); // The whole parsing process should have been successfully cancelled within a second expect(end - start).toBeLessThan(1000); }); @@ -65,7 +65,7 @@ describe('WorkerThreadAsyncParser', () => { // This file should take a few seconds to parse const file = createLargeFile(100000); const asyncParser = services.parser.AsyncParser; - const cancellationTokenSource = new CancellationTokenSource(); + const cancellationTokenSource = startCancelableOperation(); setTimeout(() => cancellationTokenSource.cancel(), 50); try { await asyncParser.parse(file, cancellationTokenSource.token); diff --git a/packages/langium/test/workspace/document-builder.test.ts b/packages/langium/test/workspace/document-builder.test.ts index 0fe779100..3829f5672 100644 --- a/packages/langium/test/workspace/document-builder.test.ts +++ b/packages/langium/test/workspace/document-builder.test.ts @@ -5,10 +5,10 @@ ******************************************************************************/ import type { AstNode, DocumentBuilder, FileSystemProvider, LangiumDocument, LangiumDocumentFactory, LangiumDocuments, Module, Reference, ValidationChecks } from 'langium'; -import { AstUtils, DocumentState, TextDocument, URI, isOperationCancelled } from 'langium'; +import { AstUtils, DocumentState, TextDocument, URI, isOperationCancelled, startCancelableOperation } from 'langium'; import { createServicesForGrammar } from 'langium/grammar'; import { describe, expect, test, vi, beforeEach, afterEach } from 'vitest'; -import { CancellationToken, CancellationTokenSource } from 'vscode-languageserver'; +import { CancellationToken } from 'vscode-languageserver'; import { fail } from 'assert'; import type { LangiumServices, LangiumSharedServices, TextDocuments } from 'langium/lsp'; @@ -147,7 +147,7 @@ describe('DefaultDocumentBuilder', () => { documents.addDocument(document2); const builder = workspace.DocumentBuilder; - const tokenSource1 = new CancellationTokenSource(); + const tokenSource1 = startCancelableOperation(); builder.onBuildPhase(DocumentState.IndexedContent, () => { tokenSource1.cancel(); }); @@ -316,7 +316,7 @@ describe('DefaultDocumentBuilder', () => { documents.addDocument(document); const actual: string[] = []; - const cancelTokenSource = new CancellationTokenSource(); + const cancelTokenSource = startCancelableOperation(); function wait(state: DocumentState): void { builder.onBuildPhase(state, async () => { actual.push('B' + state); @@ -351,7 +351,7 @@ describe('DefaultDocumentBuilder', () => { const document = documentFactory.fromString('', URI.parse('file:///test1.txt')); documents.addDocument(document); - const cancelTokenSource = new CancellationTokenSource(); + const cancelTokenSource = startCancelableOperation(); builder.waitUntil(DocumentState.IndexedReferences, cancelTokenSource.token).then(() => { fail('The test should fail here because the cancellation should reject the promise'); }).catch(err => { @@ -413,7 +413,7 @@ describe('DefaultDocumentBuilder', () => { documents.addDocument(document2); const builder = services.shared.workspace.DocumentBuilder; - const tokenSource = new CancellationTokenSource(); + const tokenSource = startCancelableOperation(); const buildPhases = new Set(); @@ -493,7 +493,7 @@ describe('DefaultDocumentBuilder', () => { `, URI.parse('file:///test1.txt')); documents.addDocument(document); - const tokenSource = new CancellationTokenSource(); + const tokenSource = startCancelableOperation(); const builder = workspace.DocumentBuilder; builder.onBuildPhase(DocumentState.ComputedScopes, () => { tokenSource.cancel(); diff --git a/packages/langium/test/workspace/workspace-lock.test.ts b/packages/langium/test/workspace/workspace-lock.test.ts index 1718d6ee9..09ff7c9f6 100644 --- a/packages/langium/test/workspace/workspace-lock.test.ts +++ b/packages/langium/test/workspace/workspace-lock.test.ts @@ -59,10 +59,10 @@ describe('WorkspaceLock', () => { test('Write action results can be awaited', async () => { const mutex = new DefaultWorkspaceLock(); - const now = Date.now(); + const now = performance.now(); const magicalNumber = await mutex.read(() => new Promise(resolve => setTimeout(() => resolve(42), 10))); // Confirm that at least 10ms have elapsed - expect(Date.now() - now).toBeGreaterThanOrEqual(10); + expect(performance.now() - now).toBeGreaterThanOrEqual(10); // Confirm the returned value expect(magicalNumber).toBe(42); }); From fea5683110c7fa6cbb9d3a73887ac31fc9f5e202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Sp=C3=B6nemann?= Date: Fri, 4 Oct 2024 06:51:57 +0000 Subject: [PATCH 2/2] Adjusted expected elapsed test time due to higher precision --- packages/langium/test/workspace/document-builder.test.ts | 2 +- packages/langium/test/workspace/workspace-lock.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/langium/test/workspace/document-builder.test.ts b/packages/langium/test/workspace/document-builder.test.ts index 3829f5672..74c64a71a 100644 --- a/packages/langium/test/workspace/document-builder.test.ts +++ b/packages/langium/test/workspace/document-builder.test.ts @@ -636,7 +636,7 @@ describe('DefaultDocumentBuilder', () => { expect(sortedDocs.slice(0, openDocumentCount).every(isDocumentOpen)).toBe(true); expect(sortedDocs.slice(openDocumentCount).every(doc => !isDocumentOpen(doc))).toBe(true); - expect(endTime - startTime).toBeLessThan(1000); // Adjust this threshold as needed + expect(endTime - startTime).toBeLessThan(1500); // Adjust this threshold as needed }); test('Sorting an empty list of documents', async () => { diff --git a/packages/langium/test/workspace/workspace-lock.test.ts b/packages/langium/test/workspace/workspace-lock.test.ts index 09ff7c9f6..5a2b15bdb 100644 --- a/packages/langium/test/workspace/workspace-lock.test.ts +++ b/packages/langium/test/workspace/workspace-lock.test.ts @@ -61,8 +61,8 @@ describe('WorkspaceLock', () => { const mutex = new DefaultWorkspaceLock(); const now = performance.now(); const magicalNumber = await mutex.read(() => new Promise(resolve => setTimeout(() => resolve(42), 10))); - // Confirm that at least 10ms have elapsed - expect(performance.now() - now).toBeGreaterThanOrEqual(10); + // Confirm that at least 5ms have elapsed + expect(performance.now() - now).toBeGreaterThanOrEqual(5); // Confirm the returned value expect(magicalNumber).toBe(42); });