From 67f1d86a965af1aeef5e75e54eff833325ccb185 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Fri, 24 Jun 2022 12:38:49 -0400 Subject: [PATCH] refactor(cache): simplify `noCache` condition (#362) - if there is no cache, we don't need to do any operations on a cache at all, so we can just totally skip all the cache operations and return early - this should be a perf improvement, at the very least on memory usage, as lots of stuff isn't created or processed now - this may make `clean: true` a more optimal choice for smaller projects, as the FS usage when writing to cache may be slower than the small amount of compilation and type-checking required in small projects - to start, from the constructor, the only necessary piece when `noCache` is the dependency tree - this is needed for `walkTree` and `setDependency` to function, but otherwise is unused when `noCache` - this _might_ be able to be further optimized to remove the graph entirely when `noCache`, but that is saved for potential future work and not part of this commit - so, we can just move the tree creation further up in the constructor as its previous ordering within the constructor does not actually matter - once this is done, we can just early return when `noCache` instead of doing all the cache-related actions, since they're not neeeded when there is no cache - no need to set `cacheDir` or any hashes etc since they're not used - note that `clean` only uses `cachePrefix` and `cacheRoot`, which are already set, and does not use `cacheDir` - no need to `init` the cache as it's not used - also slightly change the ordering to move `init` right after its prereqs are done, i.e. setting `cacheDir`, `hashOptions`, etc - just keeps with the flow instead of calling it in the middle of the ambient type processing - no need to check ambient types as that is only used for cache invalidation (marking things dirty), which is not used when there is no cache - note that `isDirty` is literally never called when `noCache` - from there, since we don't call `checkAmbientTypes` or `init` when `noCache` (the constructor is the only place they are called and they are both `private`), we can entirely remove their `noCache` branches - fairly simple for `checkAmbientTypes`, we just remove the tiny if block that sets `ambientTypesDirty`, as, well, "dirty" isn't used when there is no cache - for `init`, this means we can entirely remove the creation of `NoCache`, which isn't needed when there is no cache - that means we can also remove the implementation and tests for `NoCache` - and the reference to it in `CONTRIBUTING.md` - in `done`, we can also simply skip rolling caches and early return when there is no cache - the only other tiny change is the non-null assertions for `ambientTypes` and `cacheDir` - this matches the existing, simplifying non-null assertions for all the caches, so I did not workaround that - _could_ set a default of an empty array for `ambientTypes` etc to workaround this, but thought it better to match existing code style and not add new things - this also matches the behavior, as while `ambientTypes` and `cacheDir` could be `null`, this is only if there is no cache, in which case, they are never used --- CONTRIBUTING.md | 2 +- __tests__/nocache.spec.ts | 15 --------- src/nocache.ts | 39 --------------------- src/tscache.ts | 71 +++++++++++++++------------------------ 4 files changed, 28 insertions(+), 99 deletions(-) delete mode 100644 __tests__/nocache.spec.ts delete mode 100644 src/nocache.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dc446319..6a80cfcb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,6 +73,6 @@ A useful resource as you dive deeper are the [unit tests](__tests__/). They're g - _NOTE_: These are fairly short and unfortunately leave a lot to be desired... especially when you consider that this plugin is actually one of the simpler integrations out there. 1. At this point, you may be ready to read the more complicated bits of [`index`](src/index.ts) in detail and see how it interacts with the other modules. - The integration tests [TBD] could be useful to review at this point as well. -1. Once you're pretty familiar with `index`, you can dive into some of the cache code in [`tscache`](src/tscache.ts), [`nocache`](src/nocache.ts), and [`rollingcache`](src/rollingcache.ts). +1. Once you're pretty familiar with `index`, you can dive into some of the cache code in [`tscache`](src/tscache.ts) and [`rollingcache`](src/rollingcache.ts). 1. And finally, you can see some of the Rollup logging nuances in [`context`](src/context.ts) and [`rollupcontext`](src/rollupcontext.ts), and then the TS logging nuances in [`print-diagnostics`](src/print-diagnostics.ts), and [`diagnostics-format-host`](src/diagnostics-format-host.ts) - While these are necessary to the implementation, they are fairly ancillary to understanding and working with the codebase. diff --git a/__tests__/nocache.spec.ts b/__tests__/nocache.spec.ts deleted file mode 100644 index ee8ca11f..00000000 --- a/__tests__/nocache.spec.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { test, expect } from "@jest/globals"; - -import { NoCache } from "../src/nocache"; - -test("NoCache", () => { - const noCache = new NoCache(); - - expect(noCache.exists("")).toBeFalsy(); - expect(noCache.path("x")).toEqual("x"); - expect(noCache.match([])).toBeFalsy(); - expect(noCache.read("x")).toEqual(undefined); - expect(noCache.write("", {})).toBeFalsy(); - expect(noCache.touch("")).toBeFalsy(); - expect(noCache.roll()).toBeFalsy(); -}); diff --git a/src/nocache.ts b/src/nocache.ts deleted file mode 100644 index 8c1e15eb..00000000 --- a/src/nocache.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { ICache } from "./icache"; - -export class NoCache implements ICache -{ - public exists(_name: string): boolean - { - return false; - } - - public path(name: string): string - { - return name; - } - - public match(_names: string[]): boolean - { - return false; - } - - public read(_name: string): DataType | null | undefined - { - return undefined; - } - - public write(_name: string, _data: DataType): void - { - return; - } - - public touch(_name: string) - { - return; - } - - public roll() - { - return; - } -} diff --git a/src/tscache.ts b/src/tscache.ts index a6df796d..08f29cce 100644 --- a/src/tscache.ts +++ b/src/tscache.ts @@ -10,7 +10,6 @@ import { RollingCache } from "./rollingcache"; import { ICache } from "./icache"; import { tsModule } from "./tsproxy"; import { formatHost } from "./diagnostics-format-host"; -import { NoCache } from "./nocache"; export interface ICode { @@ -103,9 +102,9 @@ export class TsCache private cacheVersion = "9"; private cachePrefix = "rpt2_"; private dependencyTree: Graph; - private ambientTypes: ITypeSnapshot[]; + private ambientTypes!: ITypeSnapshot[]; private ambientTypesDirty = false; - private cacheDir: string | undefined; + private cacheDir!: string; private codeCache!: ICache; private typesCache!: ICache; private semanticDiagnosticsCache!: ICache; @@ -114,25 +113,28 @@ export class TsCache constructor(private noCache: boolean, hashIgnoreUnknown: boolean, private host: tsTypes.LanguageServiceHost, private cacheRoot: string, private options: tsTypes.CompilerOptions, private rollupConfig: any, rootFilenames: string[], private context: IContext) { - this.hashOptions.ignoreUnknown = hashIgnoreUnknown; - if (!noCache) + this.dependencyTree = new Graph({ directed: true }); + this.dependencyTree.setDefaultNodeLabel((_node: string) => ({ dirty: false })); + + if (noCache) { - this.cacheDir = `${this.cacheRoot}/${this.cachePrefix}${objHash( - { - version: this.cacheVersion, - rootFilenames, - options: this.options, - rollupConfig: this.rollupConfig, - tsVersion: tsModule.version, - }, - this.hashOptions, - )}`; - } else { this.clean(); + return; } - this.dependencyTree = new Graph({ directed: true }); - this.dependencyTree.setDefaultNodeLabel((_node: string) => ({ dirty: false })); + this.hashOptions.ignoreUnknown = hashIgnoreUnknown; + this.cacheDir = `${this.cacheRoot}/${this.cachePrefix}${objHash( + { + version: this.cacheVersion, + rootFilenames, + options: this.options, + rollupConfig: this.rollupConfig, + tsVersion: tsModule.version, + }, + this.hashOptions, + )}`; + + this.init(); const automaticTypes = tsModule.getAutomaticTypeDirectiveNames(options, tsModule.sys) .map((entry) => tsModule.resolveTypeReferenceDirective(entry, undefined, options, tsModule.sys)) @@ -143,8 +145,6 @@ export class TsCache .concat(automaticTypes) .map((id) => ({ id, snapshot: this.host.getScriptSnapshot(id) })); - this.init(); - this.checkAmbientTypes(); } @@ -200,6 +200,9 @@ export class TsCache public done() { + if (this.noCache) + return; + this.context.info(blue("rolling caches")); this.codeCache.roll(); this.semanticDiagnosticsCache.roll(); @@ -225,12 +228,6 @@ export class TsCache private checkAmbientTypes(): void { - if (this.noCache) - { - this.ambientTypesDirty = true; - return; - } - this.context.debug(blue("Ambient types:")); const typeHashes = this.ambientTypes.filter((snapshot) => snapshot.snapshot !== undefined) .map((snapshot) => @@ -284,24 +281,10 @@ export class TsCache private init() { - if (this.noCache) - { - this.codeCache = new NoCache(); - this.typesCache = new NoCache(); - this.syntacticDiagnosticsCache = new NoCache(); - this.semanticDiagnosticsCache = new NoCache(); - } - else - { - // this is an invariant, it should never happen: cacheDir should only not exist when noCache - if (this.cacheDir === undefined) - throw new Error(`this.cacheDir undefined`); - - this.codeCache = new RollingCache(`${this.cacheDir}/code`, true); - this.typesCache = new RollingCache(`${this.cacheDir}/types`, true); - this.syntacticDiagnosticsCache = new RollingCache(`${this.cacheDir}/syntacticDiagnostics`, true); - this.semanticDiagnosticsCache = new RollingCache(`${this.cacheDir}/semanticDiagnostics`, true); - } + this.codeCache = new RollingCache(`${this.cacheDir}/code`, true); + this.typesCache = new RollingCache(`${this.cacheDir}/types`, true); + this.syntacticDiagnosticsCache = new RollingCache(`${this.cacheDir}/syntacticDiagnostics`, true); + this.semanticDiagnosticsCache = new RollingCache(`${this.cacheDir}/semanticDiagnostics`, true); } private markAsDirty(id: string): void