fix: Improve speed of suggestions for long words. (#2406)

* fix: add operator `tap` to pipe * fix: Workaround bad node definitions * fix: calc elapsed time even for cached items. * Fix performance issue with A Start distance calc. * Add memorizer utility * Simple AutoCache * Measure the amount of time to generate a suggestion. * Update suggestCollector.test.ts.snap * Expose opTap * Update snapshots
streetsidesoftware · Feb 6, 2022 · 6c76907 · 6c76907
1 parent 1783707
commit 6c76907
Show file tree

Hide file tree

Showing 19 changed files with 347 additions and 44 deletions.
diff --git a/packages/cspell-lib/tsconfig.json b/packages/cspell-lib/tsconfig.json
@@ -1,6 +1,7 @@
 {
     "extends": "../../tsconfig.json",
     "compilerOptions": {
+        "skipLibCheck": true,
         "exactOptionalPropertyTypes": false, // make this true
         "strictFunctionTypes": false,
         "outDir": "dist"

diff --git a/packages/cspell-pipe/src/__snapshots__/index.test.ts.snap b/packages/cspell-pipe/src/__snapshots__/index.test.ts.snap
@@ -9,6 +9,7 @@ Array [
   "opFlatten",
   "opJoinStrings",
   "opMap",
+  "opTap",
   "opUnique",
   "operators",
   "pipeAsync",

diff --git a/packages/cspell-pipe/src/index.ts b/packages/cspell-pipe/src/index.ts
@@ -2,7 +2,7 @@ import * as _helpers from './helpers';
 import * as _operators from './operators';
 
 export { isAsyncIterable, toArray, toAsyncIterable } from './helpers';
-export { opAwaitAsync, opFilter, opFlatten, opJoinStrings, opMap, opUnique } from './operators';
+export { opAwaitAsync, opFilter, opFlatten, opJoinStrings, opMap, opTap, opUnique } from './operators';
 export { pipeAsync, pipeSync } from './pipe';
 
 export const operators = _operators;

diff --git a/packages/cspell-pipe/src/operators/__snapshots__/index.test.ts.snap b/packages/cspell-pipe/src/operators/__snapshots__/index.test.ts.snap
@@ -15,6 +15,9 @@ Array [
   "opMap",
   "opMapAsync",
   "opMapSync",
+  "opTap",
+  "opTapAsync",
+  "opTapSync",
   "opUnique",
   "opUniqueAsync",
   "opUniqueSync",

diff --git a/packages/cspell-pipe/src/operators/index.ts b/packages/cspell-pipe/src/operators/index.ts
@@ -3,4 +3,5 @@ export { opFilter as opFilter, opFilterAsync, opFilterSync } from './filter';
 export { opFlatten, opFlattenAsync, opFlattenSync } from './flatten';
 export { opJoinStrings, opJoinStringsAsync, opJoinStringsSync } from './joinStrings';
 export { opMap, opMapAsync, opMapSync } from './map';
+export { opTap, opTapAsync, opTapSync } from './tap';
 export { opUnique, opUniqueAsync, opUniqueSync } from './unique';
diff --git a/packages/cspell-pipe/src/operators/tap.test.ts b/packages/cspell-pipe/src/operators/tap.test.ts
@@ -0,0 +1,31 @@
+import { opMap } from '.';
+import { toArray } from '../helpers';
+import { pipeAsync, pipeSync } from '../pipe';
+import { opTap } from './tap';
+
+describe('Validate map', () => {
+    test('map', async () => {
+        const values = ['one', 'two', 'three'];
+
+        const mapFn = (v: string) => v.length;
+        const tapFn1 = jest.fn();
+        const tapFn2 = jest.fn();
+
+        const expected = values.map(mapFn);
+        const mapToLen = opMap(mapFn);
+
+        const s = pipeSync(values, opTap(tapFn1), mapToLen, opTap(tapFn2));
+        toArray(s);
+        expect(tapFn1.mock.calls.map((c) => c[0])).toEqual(values);
+        expect(tapFn2.mock.calls.map((c) => c[0])).toEqual(expected);
+
+        tapFn1.mockClear();
+        tapFn2.mockClear();
+
+        const a = pipeAsync(values, opTap(tapFn1), mapToLen, opTap(tapFn2));
+        await toArray(a);
+
+        expect(tapFn1.mock.calls.map((c) => c[0])).toEqual(values);
+        expect(tapFn2.mock.calls.map((c) => c[0])).toEqual(expected);
+    });
+});
diff --git a/packages/cspell-pipe/src/operators/tap.ts b/packages/cspell-pipe/src/operators/tap.ts
@@ -0,0 +1,39 @@
+import { toPipeFn } from '../helpers/util';
+
+/**
+ * Tap allows you to listen on values, without modifying them.
+ *
+ * @param fn - function to call for each value.
+ */
+export function opTapAsync<T>(tapFn: (v: T) => void): (iter: AsyncIterable<T>) => AsyncIterable<T> {
+    async function* fn(iter: Iterable<T> | AsyncIterable<T>) {
+        for await (const v of iter) {
+            tapFn(v);
+            yield v;
+        }
+    }
+
+    return fn;
+}
+
+/**
+ * Tap allows you to listen on values, without modifying them.
+ *
+ * @param fn - function to call for each value.
+ */
+export function opTapSync<T>(tapFn: (v: T) => void): (iter: Iterable<T>) => Iterable<T> {
+    function* fn(iter: Iterable<T>) {
+        for (const v of iter) {
+            tapFn(v);
+            yield v;
+        }
+    }
+    return fn;
+}
+
+/**
+ * Tap allows you to listen on values, without modifying them.
+ *
+ * @param fn - function to call for each value.
+ */
+export const opTap = <T>(fn: (v: T) => void) => toPipeFn<T, T>(opTapSync(fn), opTapAsync(fn));
diff --git a/packages/cspell-trie-lib/src/lib/distance/__snapshots__/distanceAStarWeighted.test.ts.snap b/packages/cspell-trie-lib/src/lib/distance/__snapshots__/distanceAStarWeighted.test.ts.snap
@@ -399,8 +399,8 @@ p: |  0|  0|  0|  0|  0|  0|  0|  0|   = 0|
 exports[`distanceAStar distanceAStarWeightedEx vs Levenshtein "grapple" "maples" 1`] = `
 "<grapple> -> <maples> (400)
 a: |<^>|<g>|<r>|<a>|<p>|<p>|<l>|<e>|<> |<$>|      |
-b: |<^>|<> |<m>|<a>|<> |<p>|<l>|<e>|<s>|<$>|      |
-c: |  0|100|100|  0|100|  0|  0|  0|100|  0| = 400|
+b: |<^>|<m>|<> |<a>|<p>|<> |<l>|<e>|<s>|<$>|      |
+c: |  0|100|100|  0|  0|100|  0|  0|100|  0| = 400|
 p: |  0|  0|  0|  0|  0|  0|  0|  0|  0|  0|   = 0|
 "
 `;

diff --git a/packages/cspell-trie-lib/src/lib/distance/distanceAStarWeighted.ts b/packages/cspell-trie-lib/src/lib/distance/distanceAStarWeighted.ts
@@ -1,3 +1,4 @@
+import assert from 'assert';
 import { PairingHeap } from '../utils/PairingHeap';
 import { WeightMap } from './weightedMaps';
 
@@ -23,14 +24,8 @@ export interface ExResult {
     }[];
 }
 
-export function distanceAStarWeightedEx(
-    wordA: string,
-    wordB: string,
-    map: WeightMap,
-    cost = 100
-): ExResult | undefined {
+export function distanceAStarWeightedEx(wordA: string, wordB: string, map: WeightMap, cost = 100): ExResult {
     const best = _distanceAStarWeightedEx(wordA, wordB, map, cost);
-    if (!best) return undefined;
 
     const aa = '^' + wordA + '$';
     const bb = '^' + wordB + '$';
@@ -56,14 +51,14 @@ export function distanceAStarWeightedEx(
     return result;
 }
 
-function _distanceAStarWeightedEx(wordA: string, wordB: string, map: WeightMap, cost = 100): Node | undefined {
+function _distanceAStarWeightedEx(wordA: string, wordB: string, map: WeightMap, cost = 100): Node {
     // Add ^ and $ for begin/end detection.
     const a = '^' + wordA + '$';
     const b = '^' + wordB + '$';
     const aN = a.length;
     const bN = b.length;
 
-    const candidates = new PairingHeap(compare);
+    const candidates = new CandidatePool(aN, bN);
 
     candidates.add({ ai: 0, bi: 0, c: 0, p: 0, f: undefined });
 
@@ -113,7 +108,7 @@ function _distanceAStarWeightedEx(wordA: string, wordB: string, map: WeightMap,
 
     let best: Node | undefined;
     // const bc2 = 2 * bc;
-    while ((best = candidates.dequeue())) {
+    while ((best = candidates.next())) {
         if (best.ai === aN && best.bi === bN) break;
 
         opSwap(best);
@@ -123,9 +118,46 @@ function _distanceAStarWeightedEx(wordA: string, wordB: string, map: WeightMap,
         opSub(best);
     }
 
+    assert(best);
     return best;
 }
 
+class CandidatePool {
+    readonly pool = new PairingHeap(compare);
+    readonly grid: Node[] = [];
+
+    constructor(readonly aN: number, readonly bN: number) {}
+
+    next(): Node | undefined {
+        let n: Node | undefined;
+        while ((n = this.pool.dequeue())) {
+            if (!n.d) return n;
+        }
+        return undefined;
+    }
+
+    add(n: NewCandidate): void {
+        const i = idx(n.ai, n.bi, this.bN);
+        const g = this.grid[i];
+        if (!g) {
+            this.grid[i] = n;
+            this.pool.add(n);
+            return;
+        }
+        // Do not add if the existing node is better.
+        if (g.c <= n.c) return;
+
+        // New node is better.
+        g.d = true;
+        this.grid[i] = n;
+        this.pool.add(n);
+    }
+}
+
+function idx(r: number, c: number, cols: number): number {
+    return r * cols + c;
+}
+
 interface Pos {
     /** the offset in string `a` */
     ai: number;
@@ -140,9 +172,15 @@ interface Node extends Pos {
     p: number;
     /** from node */
     f: Node | undefined;
+    /** deleted */
+    d?: true | undefined;
+}
+
+interface NewCandidate extends Node {
+    d?: undefined;
 }
 
 function compare(a: Node, b: Node): number {
-    // Choose lowest cost or farthest Manhattan distance.
+    // lowest cost then progress
     return a.c - b.c || b.ai + b.bi - a.ai - a.bi;
 }
diff --git a/packages/cspell-trie-lib/src/lib/suggestions/__snapshots__/suggestCollector.test.ts.snap b/packages/cspell-trie-lib/src/lib/suggestions/__snapshots__/suggestCollector.test.ts.snap
@@ -141,10 +141,10 @@ c: |  0|  0|  0|110|  0|110|  0| = 220|
 p: |  0|  0|  0|  0|  0|  0|  0|   = 0|
 ",
   "<woudt> -> <would> (220)
-a: |<^>|<w>|<o>|<u>|<> |<d>|<t>|<$>|      |
-b: |<^>|<w>|<o>|<u>|<l>|<d>|<> |<$>|      |
-c: |  0|  0|  0|  0|110|  0|110|  0| = 220|
-p: |  0|  0|  0|  0|  0|  0|  0|  0|   = 0|
+a: |<^>|<w>|<o>|<u>|<d>|<t>|<$>|      |
+b: |<^>|<w>|<o>|<u>|<l>|<d>|<$>|      |
+c: |  0|  0|  0|  0|110|110|  0| = 220|
+p: |  0|  0|  0|  0|  0|  0|  0|   = 0|
 ",
 ]
 `;
diff --git a/packages/cspell-trie-lib/src/lib/utils/autoCacheMap.test.ts b/packages/cspell-trie-lib/src/lib/utils/autoCacheMap.test.ts
@@ -0,0 +1,38 @@
+import { AutoCacheMap, AutoCacheWeakMap } from './autoCacheMap';
+import { isDefined } from './util';
+
+describe('autoCacheMap', () => {
+    test('AutoCacheMap', () => {
+        const values = ['one', 'two', 'three', 'four', 'one', 'four', 'three', 'one', 'two', 'five'];
+        const unique = [...new Set(values)];
+        function transform(s: string): number {
+            return s.length;
+        }
+
+        const fn = jest.fn(transform);
+
+        const cache = new AutoCacheMap(fn);
+
+        const r = values.map((s) => cache.get(s));
+        expect(r).toEqual(values.map(transform));
+        expect(fn.mock.calls).toEqual(unique.map((c) => [c]));
+    });
+
+    test('AutoCacheWeakMap', () => {
+        const numbers = ['one', 'two', 'three', 'four', 'one', 'four', 'three', 'one', 'two', 'five'];
+        const objs = new Map(numbers.map((s) => [s, { name: s }]));
+        const values = numbers.map((n) => objs.get(n)).filter(isDefined);
+
+        function transform(n: { name: string }): number {
+            return n.name.length;
+        }
+
+        const fn = jest.fn(transform);
+
+        const cache = new AutoCacheWeakMap(fn);
+
+        const r = values.map((s) => cache.get(s));
+        expect(r).toEqual(values.map(transform));
+        expect(fn.mock.calls).toEqual([...objs.values()].map((c) => [c]));
+    });
+});
diff --git a/packages/cspell-trie-lib/src/lib/utils/autoCacheMap.ts b/packages/cspell-trie-lib/src/lib/utils/autoCacheMap.ts
@@ -0,0 +1,31 @@
+export class AutoCacheMap<T, U> extends Map<T, U> {
+    constructor(readonly autoFn: (v: T) => U) {
+        super();
+    }
+
+    get(v: T): U {
+        const r = super.get(v);
+
+        if (r !== undefined) return r;
+
+        const u = this.autoFn(v);
+        super.set(v, u);
+        return u;
+    }
+}
+
+export class AutoCacheWeakMap<T extends object, U> extends WeakMap<T, U> {
+    constructor(readonly autoFn: (v: T) => U) {
+        super();
+    }
+
+    get(v: T): U {
+        const r = super.get(v);
+
+        if (r !== undefined) return r;
+
+        const u = this.autoFn(v);
+        super.set(v, u);
+        return u;
+    }
+}
diff --git a/packages/cspell-trie-lib/src/lib/utils/memorizer.test.ts b/packages/cspell-trie-lib/src/lib/utils/memorizer.test.ts
@@ -0,0 +1,51 @@
+import { memorizer } from './memorizer';
+
+describe('memorizer', () => {
+    test('memorizer', () => {
+        const transform = (...args: string[]) => args.reduce((sum, s) => (sum += s.length), 0);
+
+        const fn = jest.fn(transform);
+
+        const m = memorizer(fn);
+
+        const requests: string[][] = [['a'], ['a', 'b', 'c'], ['a'], ['a', 'b', 'c']];
+
+        const expected = [...new Set(requests.map((r) => r.join('|')))].map((r) => r.split('|'));
+
+        requests.forEach((r) => expect(m(...r)).toBe(transform(...r)));
+
+        expect(fn.mock.calls).toEqual(expected);
+    });
+
+    test('mixed params', () => {
+        function transform(...params: [a: string, ai: number, b: string, bi?: number | undefined]): string {
+            const [a, ai, b, bi] = params;
+            return `${a}: ${ai.toFixed(2)}, ${b}: ${bi?.toFixed(2) || '<>'}`;
+        }
+
+        const fn = jest.fn(transform);
+
+        const m = memorizer(fn);
+
+        const requests: Parameters<typeof transform>[] = [
+            ['a', 23, 'b', undefined],
+            ['a', 23, 'b'],
+            ['a', 23, 'b', undefined],
+            ['a', 23, 'b'],
+            ['a', 23, 'b'],
+            ['b', 42, 'c', 10],
+            ['a', 23, 'b', undefined],
+            ['b', 42, 'c', 10],
+        ];
+
+        const expected = [
+            ['a', 23, 'b', undefined],
+            ['a', 23, 'b'],
+            ['b', 42, 'c', 10],
+        ];
+
+        requests.forEach((r) => expect(m(...r)).toBe(transform(...r)));
+
+        expect(fn.mock.calls).toEqual(expected);
+    });
+});