Skip to content

Commit

Permalink
refactor(cache): simplify noCache condition (#362)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
agilgur5 authored Jun 24, 2022
1 parent 229dea5 commit 67f1d86
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 99 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
15 changes: 0 additions & 15 deletions __tests__/nocache.spec.ts

This file was deleted.

39 changes: 0 additions & 39 deletions src/nocache.ts

This file was deleted.

71 changes: 27 additions & 44 deletions src/tscache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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<ICode | undefined>;
private typesCache!: ICache<string>;
private semanticDiagnosticsCache!: ICache<IDiagnostics[]>;
Expand All @@ -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))
Expand All @@ -143,8 +145,6 @@ export class TsCache
.concat(automaticTypes)
.map((id) => ({ id, snapshot: this.host.getScriptSnapshot(id) }));

this.init();

this.checkAmbientTypes();
}

Expand Down Expand Up @@ -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();
Expand All @@ -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) =>
Expand Down Expand Up @@ -284,24 +281,10 @@ export class TsCache

private init()
{
if (this.noCache)
{
this.codeCache = new NoCache<ICode>();
this.typesCache = new NoCache<string>();
this.syntacticDiagnosticsCache = new NoCache<IDiagnostics[]>();
this.semanticDiagnosticsCache = new NoCache<IDiagnostics[]>();
}
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<ICode>(`${this.cacheDir}/code`, true);
this.typesCache = new RollingCache<string>(`${this.cacheDir}/types`, true);
this.syntacticDiagnosticsCache = new RollingCache<IDiagnostics[]>(`${this.cacheDir}/syntacticDiagnostics`, true);
this.semanticDiagnosticsCache = new RollingCache<IDiagnostics[]>(`${this.cacheDir}/semanticDiagnostics`, true);
}
this.codeCache = new RollingCache<ICode>(`${this.cacheDir}/code`, true);
this.typesCache = new RollingCache<string>(`${this.cacheDir}/types`, true);
this.syntacticDiagnosticsCache = new RollingCache<IDiagnostics[]>(`${this.cacheDir}/syntacticDiagnostics`, true);
this.semanticDiagnosticsCache = new RollingCache<IDiagnostics[]>(`${this.cacheDir}/semanticDiagnostics`, true);
}

private markAsDirty(id: string): void
Expand Down

0 comments on commit 67f1d86

Please sign in to comment.