Skip to content

Commit

Permalink
fix(extension 🐛): cache and extension improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
phenomnomnominal authored Jun 11, 2021
1 parent b35c773 commit 5f51b36
Show file tree
Hide file tree
Showing 19 changed files with 349 additions and 262 deletions.
8 changes: 6 additions & 2 deletions packages/betterer/src/context/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,12 @@ export class BettererContextΩ implements BettererContext {
return summary;
}

public checkCache(filePaths: BettererFilePaths): Promise<BettererFilePaths> {
return this._versionControl.checkCache(filePaths);
public checkCache(filePath: string): boolean {
return this._versionControl.checkCache(filePath);
}

public updateCache(filePaths: BettererFilePaths): void {
return this._versionControl.updateCache(filePaths);
}

private _initTests(): BettererTestMap {
Expand Down
59 changes: 35 additions & 24 deletions packages/betterer/src/fs/file-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ import { forceRelativePaths, write } from './writer';

export class BettererFileCacheΩ implements BettererFileCache {
private _cachePath: string | null = null;
private _cacheMap: BettererFileCacheMap = {};
private _diskCacheMap: BettererFileCacheMap = {};
private _memoryCacheMap: BettererFileCacheMap = {};
private _reading: Promise<string | null> | null = null;

constructor(private _versionControl: BettererVersionControl) {}

public enableCache(cachePath: string): void {
public async enableCache(cachePath: string): Promise<void> {
this._cachePath = cachePath;
this._diskCacheMap = await this._readCache(this._cachePath);
}

public async writeCache(): Promise<void> {
Expand All @@ -21,53 +23,62 @@ export class BettererFileCacheΩ implements BettererFileCache {
}

// Clean up any expired cache entries before writing to disk:
Object.keys(this._cacheMap).forEach((filePath) => {
Object.keys(this._memoryCacheMap).forEach((filePath) => {
const hash = this._versionControl.getHash(filePath);
if (hash === null) {
delete this._cacheMap[filePath];
delete this._memoryCacheMap[filePath];
}
});

const cacheString = forceRelativePaths(JSON.stringify(this._cacheMap, null, ' '), this._cachePath);
const cacheString = forceRelativePaths(JSON.stringify(this._memoryCacheMap, null, ' '), this._cachePath);
await write(cacheString, this._cachePath);
this._diskCacheMap = this._memoryCacheMap;
}

public async checkCache(filePaths: BettererFilePaths): Promise<BettererFilePaths> {
public checkCache(filePath: string): boolean {
if (!this._cachePath) {
return filePaths;
return false;
}

await this._readCache();
const hash = this._versionControl.getHash(filePath);

const notCached: Array<string> = [];
filePaths.map((filePath) => {
// If hash is null, then the file isn't tracked by version control *and* it can't be read,
// so it probably doesn't exist
if (hash === null) {
return false;
}

this._memoryCacheMap[filePath] = hash;
const previousHash = this._diskCacheMap[filePath];
return hash === previousHash;
}

public updateCache(filePaths: BettererFilePaths): void {
if (!this._cachePath) {
return;
}

filePaths.forEach((filePath) => {
const hash = this._versionControl.getHash(filePath);

// If hash is null, then the file isn't tracked by version control *and* it can't be read,
// so it probably doesn't exist
if (hash === null) {
return;
}

// If the file isn't cached, or it is cached but its contents have changed, add it to the list:
if (!this._cacheMap[filePath] || this._cacheMap[filePath] !== hash) {
notCached.push(filePath);
}
this._cacheMap[filePath] = hash;
this._memoryCacheMap[filePath] = hash;
});

return notCached;
}

private async _readCache(): Promise<void> {
if (!this._cachePath) {
return;
}
private async _readCache(cachePath: string): Promise<BettererFileCacheMap> {
if (!this._reading) {
this._reading = read(this._cachePath);
this._reading = read(cachePath);
}
const cache = await this._reading;
this._reading = null;
if (!cache) {
return;
return {};
}

const relativeCacheMap = JSON.parse(cache) as BettererFileCacheMap;
Expand All @@ -79,6 +90,6 @@ export class BettererFileCacheΩ implements BettererFileCache {
const absolutePath = path.join(path.dirname(this._cachePath as string), relativePath);
absoluteCacheMap[absolutePath] = relativeCacheMap[relativePath];
});
this._cacheMap = absoluteCacheMap;
return absoluteCacheMap;
}
}
39 changes: 22 additions & 17 deletions packages/betterer/src/fs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ export class BettererGit implements BettererVersionControl {
this._cache = new BettererFileCacheΩ(this);
}

public checkCache(filePaths: BettererFilePaths): Promise<BettererFilePaths> {
return this._cache.checkCache(filePaths);
public checkCache(filePath: string): boolean {
return this._cache.checkCache(filePath);
}

public enableCache(cachePath: string): void {
this._cache.enableCache(cachePath);
public async enableCache(cachePath: string): Promise<void> {
return this._cache.enableCache(cachePath);
}

public updateCache(filePaths: BettererFilePaths): void {
return this._cache.updateCache(filePaths);
}

public writeCache(): Promise<void> {
Expand Down Expand Up @@ -68,30 +72,31 @@ export class BettererGit implements BettererVersionControl {
return createHash(content);
}

private _toLines(output: string): Array<string> {
private _toFilePaths(output: string): Array<string> {
if (output.length === 0) {
return [];
}
return output.trimEnd().split('\n');
return Array.from(new Set(output.trimEnd().split('\n')));
}

private async _sync(): Promise<void> {
const tree = await this._git.raw(['ls-tree', '--full-tree', '-r', 'HEAD']);
const fileInfo = this._toLines(tree).map((info) => info.split(/\s/));
this._fileMap = {};
this._filePaths = fileInfo.map((fileInfo) => {
const fileInfo = this._toFilePaths(tree).map((info) => info.split(/\s/));
const gitHashes: Record<string, string> = {};
fileInfo.forEach((fileInfo) => {
const [, , hash, relativePath] = fileInfo;
const absolutePath = normalisedPath(path.join(this._rootDir, relativePath.trimStart()));
this._fileMap[absolutePath] = hash;
return absolutePath;
gitHashes[absolutePath] = hash;
});
const untracked = await this._git.raw(['ls-files', '--others', '--exclude-standard']);
// If the file isn't tracked by git, compute hashes manually
const untrackedFilePaths = this._toLines(untracked);

this._fileMap = {};
this._filePaths = [];
const modified = await this._git.raw(['ls-files', '--cached', '--modified', '--others', '--exclude-standard']);
const modifiedFilePaths = this._toFilePaths(modified);
await Promise.all(
untrackedFilePaths.map(async (relativePath) => {
modifiedFilePaths.map(async (relativePath) => {
const absolutePath = normalisedPath(path.join(this._rootDir, relativePath));
const hash = await this._getUntrackedHash(absolutePath);
const hash = gitHashes[absolutePath] || (await this._getUntrackedHash(absolutePath));
if (hash == null) {
return;
}
Expand All @@ -100,7 +105,7 @@ export class BettererGit implements BettererVersionControl {
})
);
const deleted = await this._git.raw(['ls-files', '--deleted']);
const deletedFilePaths = this._toLines(deleted);
const deletedFilePaths = this._toFilePaths(deleted);
deletedFilePaths.forEach((relativePath) => {
const absolutePath = normalisedPath(path.join(this._rootDir, relativePath));
delete this._fileMap[absolutePath];
Expand Down
5 changes: 3 additions & 2 deletions packages/betterer/src/fs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ export type BettererFilePatterns = ReadonlyArray<RegExp | ReadonlyArray<RegExp>>
export type BettererFileCacheMap = Record<string, string>;

export type BettererFileCache = {
checkCache(filePaths: BettererFilePaths): Promise<BettererFilePaths>;
enableCache(cachePath: string): void;
checkCache(filePath: string): boolean;
enableCache(cachePath: string): Promise<void>;
updateCache(fiePaths: BettererFilePaths): void;
writeCache(): Promise<void>;
};

Expand Down
2 changes: 1 addition & 1 deletion packages/betterer/src/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export async function createGlobals(
const config = await createConfig(options);
const { cache, cwd, reporters, silent } = config;
if (cache) {
versionControl.enableCache(config.cachePath);
await versionControl.enableCache(config.cachePath);
}
if (silent) {
reporter = loadReporters([]);
Expand Down
14 changes: 12 additions & 2 deletions packages/betterer/src/test/file-test/file-test-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,24 @@ import { BettererFileTestResult, BettererFileIssues, BettererFile, BettererFileB
export class BettererFileTestResultΩ implements BettererFileTestResult {
private _fileMap: Record<string, BettererFileBase> = {};
private _files: Array<BettererFileBase> = [];
private _filePaths: Array<string> = [];

constructor(private _resolver?: BettererFileResolverΩ) {}

// Previously the `files` getter was just doing `Object.values(this._fileMap)`,
// but that's pretty slow and this gets hit a lot, so instead the `this._files`
// array is populated manually in `this._addFile()`:
// array is updated manually in `this._addFile()` and `this._replaceFile()`:
public get files(): ReadonlyArray<BettererFileBase> {
return this._files;
}

// Previously the `filePaths` getter was just doing `Object.keys(this._fileMap)`,
// but that's pretty slow and this gets hit a lot, so instead the `this._filePaths`
// array is updated manually in `this._addFile()` and `this._replaceFile()`:
public get filePaths(): BettererFilePaths {
return this._filePaths;
}

public getFile(absolutePath: string): BettererFileBase {
const file = this._fileMap[absolutePath];
assert(file);
Expand All @@ -41,7 +49,7 @@ export class BettererFileTestResultΩ implements BettererFileTestResult {
}

public getFilePaths(): BettererFilePaths {
return Object.keys(this._fileMap);
return this._filePaths;
}

public getIssues(absolutePath?: string): BettererFileIssues {
Expand All @@ -54,12 +62,14 @@ export class BettererFileTestResultΩ implements BettererFileTestResult {
private _addFile(file: BettererFileBase): void {
if (!this._fileMap[file.absolutePath]) {
this._files.push(file);
this._filePaths.push(file.absolutePath);
}
this._fileMap[file.absolutePath] = file;
}

private _replaceFile(file: BettererFileBase, existingFile: BettererFileBase): void {
this._files.splice(this._files.indexOf(existingFile), 1, file);
this._filePaths.splice(this._filePaths.indexOf(existingFile.absolutePath), 1, file.absolutePath);
this._fileMap[file.absolutePath] = file;
}
}
25 changes: 18 additions & 7 deletions packages/betterer/src/test/file-test/file-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,23 +93,34 @@ function createTest(

const hasSpecifiedFiles = runΩ.filePaths?.length > 0;
runΩ.filePaths = hasSpecifiedFiles ? resolver.validate(runΩ.filePaths) : resolver.files();
const changedFiles = await contextΩ.checkCache(runΩ.filePaths);
const cacheHit = runΩ.filePaths.length !== changedFiles.length;

const expectedΩ = runΩ.isNew ? null : (runΩ.expected.value as BettererFileTestResultΩ);

let runFiles = runΩ.filePaths;
if (expectedΩ) {
runFiles = runFiles.filter((filePath) => {
const hasExpected = expectedΩ.filePaths.includes(filePath);
const isCached = contextΩ.checkCache(filePath);
return !hasExpected || !isCached;
});
}

const cacheHit = runΩ.filePaths.length !== runFiles.length;
const isPartial = hasSpecifiedFiles || cacheHit;

const result = new BettererFileTestResultΩ(resolver);
await fileTest(changedFiles, result);
await fileTest(runFiles, result);

contextΩ.updateCache(result.filePaths);

if (!isPartial || runΩ.isNew) {
if (!isPartial || !expectedΩ) {
return result;
}

const expectedΩ = runΩ.expected.value as BettererFileTestResultΩ;

// Get any filePaths that have expected issues but weren't included in this run:
const excludedFilesWithIssues = expectedΩ.files
.map((file) => file.absolutePath)
.filter((filePath) => !changedFiles.includes(filePath));
.filter((filePath) => !runFiles.includes(filePath));

// Filter them based on the current resolver:
const relevantExcludedFilePaths = resolver.validate(excludedFilesWithIssues);
Expand Down
6 changes: 6 additions & 0 deletions packages/extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@
"default": true,
"description": "Controls whether Betterer is enabled or not."
},
"betterer.cachePath": {
"scope": "resource",
"type": "string",
"default": "./.betterer.cache",
"description": "Path to Betterer cache file relative to working directory"
},
"betterer.configPath": {
"scope": "resource",
"type": "string",
Expand Down
2 changes: 1 addition & 1 deletion packages/extension/src/server/betterer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export async function hasBetterer(cwd: string): Promise<boolean> {
}

export async function getRunner(cwd: string, config: BettererOptionsRunner): Promise<BettererRunner> {
config = { ...config, cwd, silent: true };
config = { ...config, cwd, silent: true, cache: true };
const key = JSON.stringify({ ...config, cwd });
if (RUNNERS.has(key)) {
return RUNNERS.get(key) as BettererRunner;
Expand Down
4 changes: 3 additions & 1 deletion packages/extension/src/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import path from 'path';
import { RemoteWorkspace } from 'vscode-languageserver/node';

type BettererExtensionConfig = {
cachePath: string;
configPath: string;
enable: boolean;
filters: Array<string>;
Expand All @@ -30,8 +31,9 @@ export async function getDebug(workspace: RemoteWorkspace): Promise<void> {
}

export async function getBettererConfig(cwd: string, workspace: RemoteWorkspace): Promise<BettererOptionsRunner> {
const { configPath, filters, resultsPath, tsconfigPath } = await getConfig(workspace);
const { cachePath, configPath, filters, resultsPath, tsconfigPath } = await getConfig(workspace);
const config: BettererOptionsRunner = {
cachePath,
configPaths: configPath,
filters,
resultsPath
Expand Down
4 changes: 4 additions & 0 deletions packages/extension/src/server/console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,7 @@ export function initConsole(console: RemoteConsole): void {
export function info(messages: string): void {
consoleInstance.info(messages);
}

export function error(messages: string): void {
consoleInstance.error(messages);
}
1 change: 0 additions & 1 deletion packages/extension/src/server/notifications/index.ts

This file was deleted.

11 changes: 0 additions & 11 deletions packages/extension/src/server/notifications/types.ts

This file was deleted.

Loading

0 comments on commit 5f51b36

Please sign in to comment.