From 3f31569a43b9ae0f21e90d84db57f87a2cb4f6dd Mon Sep 17 00:00:00 2001 From: Jason Dent Date: Thu, 21 Dec 2023 20:54:39 +0100 Subject: [PATCH] fix: Improve performance by reducing FS reqeusts (#5103) --- packages/cspell-io/src/VirtualFS.ts | 120 +++++++++++++++--- packages/cspell-io/src/VirtualFs.test.ts | 23 ++++ .../Controller/configLoader/configLoader.ts | 2 + .../Controller/configLoader/configSearch.ts | 17 +-- .../cspell-lib/src/lib/util/resolveFile.ts | 26 +++- 5 files changed, 158 insertions(+), 30 deletions(-) diff --git a/packages/cspell-io/src/VirtualFS.ts b/packages/cspell-io/src/VirtualFS.ts index 5dfa484ec1d..a533b6337e0 100644 --- a/packages/cspell-io/src/VirtualFS.ts +++ b/packages/cspell-io/src/VirtualFS.ts @@ -16,6 +16,8 @@ type UrlOrReference = URL | FileReference; type NextProvider = (url: URL) => VProviderFileSystem | undefined; +const debug = false; + export interface VirtualFS extends Disposable { registerFileSystemProvider(provider: VFileSystemProvider, ...providers: VFileSystemProvider[]): Disposable; /** @@ -32,6 +34,13 @@ export interface VirtualFS extends Disposable { * Clear the cache of file systems. */ reset(): void; + + /** + * Indicates that logging has been enabled. + */ + loggingEnabled: boolean; + + enableLogging(value?: boolean): void; } export enum FSCapabilityFlags { @@ -131,11 +140,27 @@ class CVirtualFS implements VirtualFS { private cachedFs = new Map(); private revCacheFs = new Map>(); readonly fs: Required; + loggingEnabled = debug; constructor() { this.fs = fsPassThrough((url) => this._getFS(url)); } + enableLogging(value?: boolean | undefined): void { + this.loggingEnabled = value ?? true; + } + + log = console.log; + logEvent = (event: LogEvent) => { + if (this.loggingEnabled) { + const id = event.traceID.toFixed(13).replace(/\d{4}(?=\d)/g, '$&.'); + const msg = event.message ? `\n\t\t${event.message}` : ''; + this.log( + `${event.method}-${event.event}\t ID:${id} ts:${event.ts.toFixed(13)} ${chopUrl(event.url)}${msg}`, + ); + } + }; + registerFileSystemProvider(...providers: VFileSystemProvider[]): Disposable { providers.forEach((provider) => this.providers.add(provider)); this.reset(); @@ -190,7 +215,7 @@ class CVirtualFS implements VirtualFS { next = fnNext(provider, next); } - const fs = new WrappedProviderFs(next(url)); + const fs = new WrappedProviderFs(next(url), this.logEvent); this.cachedFs.set(key, fs); return fs; } @@ -361,69 +386,112 @@ export function fsCapabilities(flags: FSCapabilityFlags): FSCapabilities { return new CFsCapabilities(flags); } +type EventMethods = 'stat' | 'readFile' | 'writeFile' | 'readDir'; +type LogEvents = 'start' | 'end' | 'error' | 'other'; + +interface LogEvent { + /** + * The request method + */ + method: EventMethods; + event: LogEvents; + message?: string | undefined; + /** + * The trace id can be used to link request and response events. + * The trace id is unique for a given request. + */ + traceID: number; + /** + * The request url + */ + url?: URL | undefined; + /** + * The time in milliseconds, see `performance.now()` + */ + ts: number; +} + class WrappedProviderFs implements VFileSystem { readonly hasProvider: boolean; readonly capabilities: FSCapabilityFlags; readonly providerInfo: FileSystemProviderInfo; private _capabilities: FSCapabilities; - constructor(private readonly fs: VProviderFileSystem | undefined) { + constructor( + private readonly fs: VProviderFileSystem | undefined, + readonly eventLogger: (event: LogEvent) => void, + ) { this.hasProvider = !!fs; this.capabilities = fs?.capabilities || FSCapabilityFlags.None; this._capabilities = fsCapabilities(this.capabilities); this.providerInfo = fs?.providerInfo || { name: 'unknown' }; } + private logEvent(method: EventMethods, event: LogEvents, traceID: number, url: URL, message?: string): void { + this.eventLogger({ method, event, url, traceID, ts: performance.now(), message }); + } + getCapabilities(url: URL): FSCapabilities { if (this.fs?.getCapabilities) return this.fs.getCapabilities(url); return this._capabilities; } - async stat(url: UrlOrReference): Promise { + async stat(urlRef: UrlOrReference): Promise { + const traceID = performance.now(); + const url = urlOrReferenceToUrl(urlRef); + this.logEvent('stat', 'start', traceID, url); try { - checkCapabilityOrThrow( - this.fs, - this.capabilities, - FSCapabilityFlags.Stat, - 'stat', - urlOrReferenceToUrl(url), - ); - return new CVfsStat(await this.fs.stat(url)); + checkCapabilityOrThrow(this.fs, this.capabilities, FSCapabilityFlags.Stat, 'stat', url); + return new CVfsStat(await this.fs.stat(urlRef)); } catch (e) { + this.logEvent('stat', 'error', traceID, url, e instanceof Error ? e.message : ''); throw wrapError(e); + } finally { + this.logEvent('stat', 'end', traceID, url); } } - async readFile(url: UrlOrReference, encoding?: BufferEncoding): Promise { + async readFile(urlRef: UrlOrReference, encoding?: BufferEncoding): Promise { + const traceID = performance.now(); + const url = urlOrReferenceToUrl(urlRef); + this.logEvent('readFile', 'start', traceID, url); try { - checkCapabilityOrThrow( - this.fs, - this.capabilities, - FSCapabilityFlags.Read, - 'readFile', - urlOrReferenceToUrl(url), - ); - return createTextFileResource(await this.fs.readFile(url), encoding); + checkCapabilityOrThrow(this.fs, this.capabilities, FSCapabilityFlags.Read, 'readFile', url); + return createTextFileResource(await this.fs.readFile(urlRef), encoding); } catch (e) { + this.logEvent('readFile', 'error', traceID, url, e instanceof Error ? e.message : ''); throw wrapError(e); + } finally { + this.logEvent('readFile', 'end', traceID, url); } } async readDirectory(url: URL): Promise { + const traceID = performance.now(); + this.logEvent('readDir', 'start', traceID, url); try { checkCapabilityOrThrow(this.fs, this.capabilities, FSCapabilityFlags.ReadDir, 'readDirectory', url); return (await this.fs.readDirectory(url)).map((e) => new CVfsDirEntry(e)); } catch (e) { + this.logEvent('readDir', 'error', traceID, url, e instanceof Error ? e.message : ''); throw wrapError(e); + } finally { + this.logEvent('readDir', 'end', traceID, url); } } async writeFile(file: FileResource): Promise { + const traceID = performance.now(); + const url = file.url; + this.logEvent('writeFile', 'start', traceID, url); try { checkCapabilityOrThrow(this.fs, this.capabilities, FSCapabilityFlags.Write, 'writeFile', file.url); return await this.fs.writeFile(file); } catch (e) { + this.logEvent('writeFile', 'error', traceID, url, e instanceof Error ? e.message : ''); throw wrapError(e); + } finally { + this.logEvent('writeFile', 'end', traceID, url); } } @@ -510,3 +578,15 @@ class CVfsDirEntry extends CFileType implements VfsDirEntry { return this._url; } } + +function chopUrl(url: URL | undefined): string { + if (!url) return ''; + const href = url.href; + const parts = href.split('/'); + const n = parts.indexOf('node_modules'); + if (n > 0) { + const tail = parts.slice(Math.max(parts.length - 3, n + 1)); + return parts.slice(0, n + 1).join('/') + '/.../' + tail.join('/'); + } + return href; +} diff --git a/packages/cspell-io/src/VirtualFs.test.ts b/packages/cspell-io/src/VirtualFs.test.ts index 97852ddcdca..dc135ee4860 100644 --- a/packages/cspell-io/src/VirtualFs.test.ts +++ b/packages/cspell-io/src/VirtualFs.test.ts @@ -11,15 +11,19 @@ import { createVirtualFS, FSCapabilityFlags, getDefaultVirtualFs, VFSErrorUnsupp const sc = expect.stringContaining; const oc = expect.objectContaining; +let mockConsoleLog = vi.spyOn(console, 'log'); + describe('VirtualFs', () => { let virtualFs: VirtualFS; beforeEach(() => { virtualFs = createVirtualFS(); + mockConsoleLog = vi.spyOn(console, 'log'); }); afterEach(() => { virtualFs.dispose(); + vi.resetAllMocks(); }); test('should create a file system', () => { @@ -194,11 +198,30 @@ describe('VirtualFs', () => { url = toFileURL(url); const fs = getDefaultVirtualFs().fs; const s = await fs.stat(url); + expect(mockConsoleLog).not.toHaveBeenCalled(); + expect(s).toEqual(expected); + expect(s.isFile()).toBe(true); + expect(s.isDirectory()).toBe(false); + expect(s.isUnknown()).toBe(false); + expect(s.eTag).toBeUndefined(); + }); + + test.each` + url | expected + ${__filename} | ${oc({ mtimeMs: expect.any(Number), size: expect.any(Number), fileType: 1 })} + `('getStat with logging $url', async ({ url, expected }) => { + url = toFileURL(url); + const virtualFs = createVirtualFS(); + virtualFs.enableLogging(); + const fs = virtualFs.fs; + const s = await fs.stat(url); + expect(mockConsoleLog).toHaveBeenCalledTimes(2); expect(s).toEqual(expected); expect(s.isFile()).toBe(true); expect(s.isDirectory()).toBe(false); expect(s.isUnknown()).toBe(false); expect(s.eTag).toBeUndefined(); + virtualFs.dispose(); }); test.each` diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts index e2c1fc63829..3e5fd6148d1 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts @@ -506,6 +506,8 @@ export class ConfigLoader implements IConfigLoader { private async resolveFilename(filename: string | URL, relativeTo: string | URL): Promise { if (filename instanceof URL) return { filename: toFilePathOrHref(filename) }; + if (isUrlLike(filename)) return { filename: toFilePathOrHref(filename) }; + const r = await this.fileResolver.resolveFile(filename, relativeTo); if (r.warning) { diff --git a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configSearch.ts b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configSearch.ts index 8f3b5e1f1fb..b989afad079 100644 --- a/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configSearch.ts +++ b/packages/cspell-lib/src/lib/Settings/Controller/configLoader/configSearch.ts @@ -82,6 +82,15 @@ export class ConfigSearch { const hasFile = async (filename: URL): Promise => { const dir = new URL('.', filename); + const parent = new URL('..', dir); + const parentHref = parent.href; + const parentInfoP = dirInfoCache.get(parentHref); + if (parentInfoP) { + const parentInfo = await parentInfoP; + const name = urlBasename(dir).slice(0, -1); + const found = parentInfo.get(name); + if (!found?.isDirectory()) return false; + } const dirUrlHref = dir.href; const dirInfo = await dirInfoCache.get( dirUrlHref, @@ -131,11 +140,3 @@ async function checkPackageJson(fs: VFileSystem, filename: URL): Promise { -// try { -// return (await virtualFs.fs.stat(path)).isDirectory(); -// } catch (e) { -// return false; -// } -// } diff --git a/packages/cspell-lib/src/lib/util/resolveFile.ts b/packages/cspell-lib/src/lib/util/resolveFile.ts index 4e105e04f3b..2bba4dd414d 100644 --- a/packages/cspell-lib/src/lib/util/resolveFile.ts +++ b/packages/cspell-lib/src/lib/util/resolveFile.ts @@ -83,7 +83,7 @@ export class FileResolver { filename: string; fn: (f: string, r: string | URL) => Promise | ResolveFileResult | undefined; }[] = [ - { filename, fn: this.tryUrl }, + { filename, fn: this.tryUrlRel }, { filename, fn: this.tryCreateRequire }, { filename, fn: this.tryNodeRequireResolve }, { filename, fn: this.tryImportResolve }, @@ -125,7 +125,7 @@ export class FileResolver { * @param filename - url string * @returns ResolveFileResult */ - tryUrl = async (filename: string, relativeToURL: string | URL): Promise => { + tryUrlRel = async (filename: string, relativeToURL: string | URL): Promise => { if (isURLLike(filename)) { const fileURL = toURL(filename); return { @@ -136,6 +136,28 @@ export class FileResolver { }; } + if (isRelative(filename) && isURLLike(relativeToURL) && !isDataURL(relativeToURL)) { + const relToURL = toURL(relativeToURL); + const url = resolveFileWithURL(filename, relToURL); + return { + filename: toFilePathOrHref(url), + relativeTo: toFilePathOrHref(relToURL), + found: await this.doesExist(url), + method: 'tryUrl', + }; + } + + return undefined; + }; + + /** + * Check to see if it is a URL. + * Note: URLs are absolute! + * If relativeTo is a non-file URL, then it will try to resolve the filename relative to it. + * @param filename - url string + * @returns ResolveFileResult + */ + tryUrl = async (filename: string, relativeToURL: string | URL): Promise => { if (isURLLike(relativeToURL) && !isDataURL(relativeToURL)) { const relToURL = toURL(relativeToURL); const url = resolveFileWithURL(filename, relToURL);