Skip to content

Commit

Permalink
dev: eslint-plugin improve suggestion speed (#2590)
Browse files Browse the repository at this point in the history
* dev: Make suggestion generation lazy
* dev: work towards faster suggestions
   Work towards being able to cache suggestion lookup.
  • Loading branch information
Jason3S committed Mar 15, 2022
1 parent aaffda7 commit 3ac2ba1
Show file tree
Hide file tree
Showing 14 changed files with 167 additions and 94 deletions.
21 changes: 16 additions & 5 deletions packages/cspell-lib/src/Settings/DictionarySettings.test.ts
Expand Up @@ -7,6 +7,7 @@ import { getDefaultSettings } from './DefaultSettings';
import * as DictSettings from './DictionarySettings';

const defaultSettings = getDefaultSettings();
const oc = expect.objectContaining;

describe('Validate DictionarySettings', () => {
test('expects default to not be empty', () => {
Expand Down Expand Up @@ -79,11 +80,13 @@ describe('Validate DictionarySettings', () => {
};

const nDef = DictSettings.mapDictDefToInternal(def, pathToConfig);
expect(nDef).toEqual({
name: 'words',
path: absolutePath,
__source: pathToConfig,
});
expect(nDef).toEqual(
oc({
name: 'words',
path: absolutePath,
__source: pathToConfig,
})
);

const legacyDef: DictionaryDefinitionLegacy = {
name: 'words',
Expand Down Expand Up @@ -114,4 +117,12 @@ describe('Validate DictionarySettings', () => {
'Trying to normalize a dictionary definition with a different source.'
);
});

test.each`
def | expected
${{}} | ${false}
${DictSettings.mapDictDefToInternal({ name: 'def', path: './dict.txt' }, __filename)} | ${true}
`('isDictionaryDefinitionInternal', ({ def, expected }) => {
expect(DictSettings.isDictionaryDefinitionInternal(def)).toBe(expected);
});
});
32 changes: 17 additions & 15 deletions packages/cspell-lib/src/Settings/DictionarySettings.ts
@@ -1,7 +1,6 @@
import type {
CustomDictionaryScope,
DictionaryDefinition,
DictionaryDefinitionPreferred,
DictionaryDefinitionAugmented,
DictionaryDefinitionCustom,
DictionaryFileTypes,
Expand Down Expand Up @@ -39,26 +38,23 @@ export function filterDictDefsToLoad(
dictRefIds: DictionaryReference[],
defs: DictionaryDefinitionInternal[]
): DictionaryDefinitionInternal[] {
function isDefP(def: DictionaryDefinition): def is DictionaryDefinitionPreferred {
return !!def.path;
}

const col = createDictionaryReferenceCollection(dictRefIds);
const dictIdSet = new Set(col.enabled());
const allActiveDefs = defs
.filter(({ name }) => dictIdSet.has(name))
.map((def) => ({ ...def, path: getFullPathName(def) }))
// Remove any empty paths.
.filter(isDefP);
const allActiveDefs = defs.filter(({ name }) => dictIdSet.has(name)).map(fixPath);
return [...new Map(allActiveDefs.map((d) => [d.name, d])).values()];
}

function getFullPathName(def: DictionaryDefinition) {
const { path: filePath = '', file = '' } = def;
if (!filePath && !file) {
return '';
function fixPath(def: DictionaryDefinitionInternal): DictionaryDefinitionInternal {
if (def instanceof _DictionaryDefinitionInternalWithSource) {
return def;
}
return path.join(filePath, file);
const { path: filePath = '', file = '' } = def;
const newPath = !filePath && !file ? '' : path.join(filePath, file);
return {
...def,
file: undefined,
path: newPath,
};
}

export function mapDictDefsToInternal(defs: undefined, pathToSettingsFile: string): undefined;
Expand Down Expand Up @@ -117,6 +113,12 @@ type DictDef = Partial<
UnionFields<UnionFields<DictionaryDefinition, DictionaryDefinitionAugmented>, DictionaryDefinitionCustom>
>;

export function isDictionaryDefinitionInternal(
def: DictionaryDefinition | DictionaryDefinitionInternal
): def is DictionaryDefinitionInternal {
return def instanceof _DictionaryDefinitionInternalWithSource;
}

class _DictionaryDefinitionInternalWithSource implements DictionaryDefinitionInternalWithSource {
private _weightMap: WeightMap | undefined;
readonly name: string;
Expand Down
30 changes: 7 additions & 23 deletions packages/cspell-lib/src/SpellingDictionary/Dictionaries.test.ts
Expand Up @@ -3,7 +3,7 @@ import * as fs from 'fs-extra';
import * as path from 'path';
import { createCSpellSettingsInternal as csi } from '../Models/CSpellSettingsInternalDef';
import { getDefaultSettings, loadConfig } from '../Settings';
import { filterDictDefsToLoad } from '../Settings/DictionarySettings';
import { filterDictDefsToLoad, mapDictDefToInternal } from '../Settings/DictionarySettings';
import * as Dictionaries from './Dictionaries';
import { __testing__ } from './DictionaryLoader';
import { isSpellingDictionaryLoadError } from './SpellingDictionaryError';
Expand All @@ -22,6 +22,8 @@ function log(msg: string): void {
}
}

const di = mapDictDefToInternal;

describe('Validate getDictionary', () => {
const ignoreCaseFalse = { ignoreCase: false };
const ignoreCaseTrue = { ignoreCase: true };
Expand Down Expand Up @@ -136,9 +138,8 @@ describe('Validate getDictionary', () => {
});

test('Dictionary NOT Found', async () => {
const weightMap = undefined;
const settings = csi({
dictionaryDefinitions: [{ name: 'my-words', path: './not-found.txt', weightMap, __source: undefined }],
dictionaryDefinitions: [di({ name: 'my-words', path: './not-found.txt' }, __filename)],
dictionaries: ['my-words'],
});

Expand Down Expand Up @@ -184,21 +185,10 @@ describe('Validate Refresh', () => {
const tempDictPathNotFound = tempPath('not-found.txt');
await fs.mkdirp(path.dirname(tempDictPath));
await fs.writeFile(tempDictPath, 'one\ntwo\nthree\n');
const weightMap = undefined;
const settings = getDefaultSettings();
const defs = (settings.dictionaryDefinitions || []).concat([
{
name: 'temp',
path: tempDictPath,
weightMap,
__source: undefined,
},
{
name: 'not_found',
path: tempDictPathNotFound,
weightMap,
__source: undefined,
},
di({ name: 'temp', path: tempDictPath }, __filename),
di({ name: 'not_found', path: tempDictPathNotFound }, __filename),
]);
const toLoad = ['node', 'html', 'css', 'not_found', 'temp'];
const defsToLoad = filterDictDefsToLoad(toLoad, defs);
Expand Down Expand Up @@ -242,15 +232,9 @@ describe('Validate Refresh', () => {
const tempDictPath = tempPath('words_sync.txt');
await fs.mkdirp(path.dirname(tempDictPath));
await fs.writeFile(tempDictPath, 'one\ntwo\nthree\n');
const weightMap = undefined;
const settings = getDefaultSettings();
const defs = (settings.dictionaryDefinitions || []).concat([
{
name: 'temp',
path: tempDictPath,
weightMap,
__source: undefined,
},
di({ name: 'temp', path: tempDictPath }, __filename),
]);
const toLoad = ['node', 'html', 'css', 'temp'];
const defsToLoad = filterDictDefsToLoad(toLoad, defs);
Expand Down
4 changes: 2 additions & 2 deletions packages/cspell-lib/src/SpellingDictionary/Dictionaries.ts
Expand Up @@ -7,11 +7,11 @@ import { SpellingDictionary } from './SpellingDictionary';
import { createCollection, SpellingDictionaryCollection } from './SpellingDictionaryCollection';

export function loadDictionaryDefs(defsToLoad: DictionaryDefinitionInternal[]): Promise<SpellingDictionary>[] {
return defsToLoad.map((def) => loadDictionary(def.path, def));
return defsToLoad.map(loadDictionary);
}

export function loadDictionaryDefsSync(defsToLoad: DictionaryDefinitionInternal[]): SpellingDictionary[] {
return defsToLoad.map((def) => loadDictionarySync(def.path, def));
return defsToLoad.map(loadDictionarySync);
}

export function refreshDictionaryCache(maxAge?: number): Promise<void> {
Expand Down
20 changes: 10 additions & 10 deletions packages/cspell-lib/src/SpellingDictionary/DictionaryLoader.test.ts
@@ -1,5 +1,6 @@
import * as path from 'path';
import { DictionaryDefinitionInternal } from '../Models/CSpellSettingsInternalDef';
import { mapDictDefToInternal } from '../Settings/DictionarySettings';
import { clean } from '../util/util';
import { loadDictionary, loadDictionarySync, LoadOptions, refreshCacheEntries, testing } from './DictionaryLoader';
jest.mock('../util/logger');
Expand All @@ -9,6 +10,8 @@ const samples = path.join(root, 'samples');

type ErrorResults = Record<string, unknown> | Error;

const di = mapDictDefToInternal;

describe('Validate DictionaryLoader', () => {
const errorENOENT = { code: 'ENOENT' };
const unknownFormatError = new Error('Unknown file format');
Expand Down Expand Up @@ -63,7 +66,7 @@ describe('Validate DictionaryLoader', () => {
path: filename,
name: filename,
});
const dict = await loadDictionary(filename, def);
const dict = await loadDictionary(def);
expect(dict.getErrors?.()).toHaveLength(1);
});

Expand Down Expand Up @@ -130,7 +133,8 @@ describe('Validate DictionaryLoader', () => {
hasErrors: boolean;
}) => {
await refreshCacheEntries(maxAge, Date.now());
const d = await loadDictionary(file, options);
const def = { ...options, path: file };
const d = await loadDictionary(def);
expect(d.has(word)).toBe(hasWord);
expect(!!d.getErrors?.().length).toBe(hasErrors);
}
Expand All @@ -155,7 +159,7 @@ describe('Validate DictionaryLoader', () => {
'dict has word $testCase $word',
async ({ word, hasWord, ignoreCase }: { word: string; hasWord: boolean; ignoreCase?: boolean }) => {
const file = sample('words.txt');
const d = await loadDictionary(file, dDef({ name: 'words', path: file }));
const d = await loadDictionary(dDef({ name: 'words', path: file }));
expect(d.has(word, clean({ ignoreCase }))).toBe(hasWord);
}
);
Expand All @@ -173,7 +177,7 @@ describe('Validate DictionaryLoader', () => {
${dDef({ name: 'words', path: dict('cities.txt'), type: 'W' })} | ${'New York'} | ${false}
${dDef({ name: 'words', path: dict('cities.txt'), type: 'W' })} | ${'York'} | ${true}
`('sync load dict has word $def $word', ({ def, word, hasWord }) => {
const d = loadDictionarySync(def.path, def);
const d = loadDictionarySync(def);
expect(d.has(word)).toBe(hasWord);
});

Expand All @@ -185,7 +189,7 @@ describe('Validate DictionaryLoader', () => {
${dDef({ name: 'words', path: dict('cities.missing.txt'), type: 'S' })} | ${[expect.any(Error)]}
${dDef({ name: 'words', path: dict('cities.missing.txt'), type: 'W' })} | ${[expect.any(Error)]}
`('sync load dict with error $def', ({ def, expected }) => {
const d = loadDictionarySync(def.path, def);
const d = loadDictionarySync(def);
expect(d.getErrors?.()).toEqual(expected);
});

Expand All @@ -207,9 +211,5 @@ interface DDef extends Partial<DictionaryDefinitionInternal> {
}

function dDef(opts: DDef): DictionaryDefinitionInternal {
return {
weightMap: undefined,
__source: '',
...opts,
};
return di(opts, __filename);
}
66 changes: 36 additions & 30 deletions packages/cspell-lib/src/SpellingDictionary/DictionaryLoader.ts
Expand Up @@ -79,51 +79,54 @@ export interface SyncLoaders {
}

const dictionaryCache = new Map<string, CacheEntry>();
const dictionaryCacheByDef = new Map<DictionaryDefinitionInternal, { key: string; entry: CacheEntry }>();

export function loadDictionary(uri: string, options: DictionaryDefinitionInternal): Promise<SpellingDictionary> {
const key = calcKey(uri, options);
function getCacheEntry(def: DictionaryDefinitionInternal): { key: string; entry: CacheEntry | undefined } {
const defEntry = dictionaryCacheByDef.get(def);
if (defEntry) {
return defEntry;
}
const key = calcKey(def);
const entry = dictionaryCache.get(key);
if (entry) {
// replace old entry so it can be released.
entry.options = def;
}
return { key, entry };
}

function setCacheEntry(key: string, entry: CacheEntry, def: DictionaryDefinitionInternal) {
dictionaryCache.set(key, entry);
dictionaryCacheByDef.set(def, { key, entry });
}

export function loadDictionary(def: DictionaryDefinitionInternal): Promise<SpellingDictionary> {
const { key, entry } = getCacheEntry(def);
if (entry) {
return entry.pending.then(([dictionary]) => dictionary);
}
const loadedEntry = loadEntry(uri, options);
dictionaryCache.set(key, loadedEntry);
const loadedEntry = loadEntry(def.path, def);
setCacheEntry(key, loadedEntry, def);
return loadedEntry.pending.then(([dictionary]) => dictionary);
}

export function loadDictionarySync(uri: string, options: DictionaryDefinitionInternal): SpellingDictionary {
const key = calcKey(uri, options);
const entry = dictionaryCache.get(key);
export function loadDictionarySync(def: DictionaryDefinitionInternal): SpellingDictionary {
const { key, entry } = getCacheEntry(def);
if (entry?.dictionary && entry.loadingState === LoadingState.Loaded) {
// if (entry.options.name === 'temp') {
// __log(
// `Cache Found ${entry.options.name}; ts: ${entry.sig.toFixed(2)}; file: ${path.relative(
// process.cwd(),
// entry.uri
// )}`
// );
// }
return entry.dictionary;
}
// if (options.name === 'temp') {
// __log(
// `Cache Miss ${options.name}; ts: ${entry?.sig.toFixed(2) || Date.now()}; file: ${path.relative(
// process.cwd(),
// uri
// )}`
// );
// }
const loadedEntry = loadEntrySync(uri, options);
dictionaryCache.set(key, loadedEntry);
const loadedEntry = loadEntrySync(def.path, def);
setCacheEntry(key, loadedEntry, def);
return loadedEntry.dictionary;
}

const importantOptionKeys: (keyof DictionaryDefinitionInternal)[] = ['name', 'noSuggest', 'useCompounds', 'type'];

function calcKey(uri: string, options: DictionaryDefinitionInternal) {
const loaderType = determineType(uri, options);
const optValues = importantOptionKeys.map((k) => options[k]?.toString() || '');
const parts = [uri, loaderType].concat(optValues);
function calcKey(def: DictionaryDefinitionInternal) {
const path = def.path;
const loaderType = determineType(path, def);
const optValues = importantOptionKeys.map((k) => def[k]?.toString() || '');
const parts = [path, loaderType].concat(optValues);

return parts.join('|');
}
Expand Down Expand Up @@ -160,7 +163,10 @@ async function refreshEntry(entry: CacheEntry, maxAge: number, now: number): Pro
// }
if (sigMatches && hasChanged) {
entry.loadingState = LoadingState.Loading;
dictionaryCache.set(calcKey(entry.uri, entry.options), loadEntry(entry.uri, entry.options));
const key = calcKey(entry.options);
const newEntry = loadEntry(entry.uri, entry.options);
dictionaryCache.set(key, newEntry);
dictionaryCacheByDef.set(entry.options, { key, entry: newEntry });
}
}
}
Expand Down
@@ -1,5 +1,6 @@
import * as Trie from 'cspell-trie-lib';
import { SpellingDictionaryOptions } from '.';
import { mapDictDefToInternal } from '../Settings/DictionarySettings';
import {
createFailedToLoadDictionary,
createForbiddenWordsDictionary,
Expand All @@ -10,6 +11,8 @@ import { createCollection, SpellingDictionaryCollection } from './SpellingDictio
import { SpellingDictionaryLoadError } from './SpellingDictionaryError';
import { SpellingDictionaryFromTrie } from './SpellingDictionaryFromTrie';

const di = mapDictDefToInternal;

describe('Verify using multiple dictionaries', () => {
const wordsA = [
'',
Expand Down Expand Up @@ -75,7 +78,7 @@ describe('Verify using multiple dictionaries', () => {
createFailedToLoadDictionary(
new SpellingDictionaryLoadError(
'./missing.txt',
{ name: 'error', path: './missing.txt', weightMap: undefined, __source: '' },
di({ name: 'error', path: './missing.txt' }, __filename),
new Error('error'),
'failed to load'
)
Expand Down
@@ -1,14 +1,17 @@
import { SpellingDictionaryOptions } from '.';
import { DictionaryInformation } from '..';
import { mapDictDefToInternal } from '../Settings/DictionarySettings';
import { createFailedToLoadDictionary, createSpellingDictionary } from './createSpellingDictionary';
import { SpellingDictionaryLoadError } from './SpellingDictionaryError';

const di = mapDictDefToInternal;

describe('Validate createSpellingDictionary', () => {
test('createFailedToLoadDictionary', () => {
const error = new Error('error');
const loaderError = new SpellingDictionaryLoadError(
'./missing.txt',
{ name: 'failed dict', path: './missing.txt', weightMap: undefined, __source: undefined },
di({ name: 'failed dict', path: './missing.txt' }, __filename),
error,
'Failed to load'
);
Expand Down

0 comments on commit 3ac2ba1

Please sign in to comment.