Skip to content

Commit

Permalink
fix: Improve performance by reducing FS reqeusts (#5103)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Jason3S
Jason3S authored Dec 21, 2023
1 parent c0c3427 commit 3f31569
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 30 deletions.
120 changes: 100 additions & 20 deletions packages/cspell-io/src/VirtualFS.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/**
Expand All @@ -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 {
Expand Down Expand Up @@ -131,11 +140,27 @@ class CVirtualFS implements VirtualFS {
private cachedFs = new Map<string, WrappedProviderFs>();
private revCacheFs = new Map<VFileSystemProvider, Set<string>>();
readonly fs: Required<VFileSystem>;
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();
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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<VfsStat> {
async stat(urlRef: UrlOrReference): Promise<VfsStat> {
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<TextFileResource> {
async readFile(urlRef: UrlOrReference, encoding?: BufferEncoding): Promise<TextFileResource> {
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<VfsDirEntry[]> {
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<FileReference> {
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);
}
}

Expand Down Expand Up @@ -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;
}
23 changes: 23 additions & 0 deletions packages/cspell-io/src/VirtualFs.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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`
Expand Down
2 changes: 2 additions & 0 deletions packages/cspell-lib/src/lib/Settings/Controller/configLoader/configLoader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,8 @@ export class ConfigLoader implements IConfigLoader {

private async resolveFilename(filename: string | URL, relativeTo: string | URL): Promise<ImportFileRef> {
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) {
Expand Down
17 changes: 9 additions & 8 deletions packages/cspell-lib/src/lib/Settings/Controller/configLoader/configSearch.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ export class ConfigSearch {

const hasFile = async (filename: URL): Promise<boolean> => {
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,
Expand Down Expand Up @@ -131,11 +140,3 @@ async function checkPackageJson(fs: VFileSystem, filename: URL): Promise<boolean
return false;
}
}

// async function isDirectory(virtualFs: VirtualFS, path: URL): Promise<boolean> {
// try {
// return (await virtualFs.fs.stat(path)).isDirectory();
// } catch (e) {
// return false;
// }
// }
26 changes: 24 additions & 2 deletions packages/cspell-lib/src/lib/util/resolveFile.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export class FileResolver {
filename: string;
fn: (f: string, r: string | URL) => Promise<ResolveFileResult | undefined> | ResolveFileResult | undefined;
}[] = [
{ filename, fn: this.tryUrl },
{ filename, fn: this.tryUrlRel },
{ filename, fn: this.tryCreateRequire },
{ filename, fn: this.tryNodeRequireResolve },
{ filename, fn: this.tryImportResolve },
Expand Down Expand Up @@ -125,7 +125,7 @@ export class FileResolver {
* @param filename - url string
* @returns ResolveFileResult
*/
tryUrl = async (filename: string, relativeToURL: string | URL): Promise<ResolveFileResult | undefined> => {
tryUrlRel = async (filename: string, relativeToURL: string | URL): Promise<ResolveFileResult | undefined> => {
if (isURLLike(filename)) {
const fileURL = toURL(filename);
return {
Expand All @@ -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<ResolveFileResult | undefined> => {
if (isURLLike(relativeToURL) && !isDataURL(relativeToURL)) {
const relToURL = toURL(relativeToURL);
const url = resolveFileWithURL(filename, relToURL);
Expand Down

0 comments on commit 3f31569

Please sign in to comment.