Add back cache to VSCode scorer. (#2126)

* Add a cache back. There is a todo explaining possible further work, after testing. * Fix tests. * Fix lint issues.
onivim · Apr 24, 2018 · ba5e4de · ba5e4de
1 parent 7fc14d0
commit ba5e4de
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 24 deletions.
diff --git a/browser/src/Services/QuickOpen/Scorer/OniQuickOpenScorer.ts b/browser/src/Services/QuickOpen/Scorer/OniQuickOpenScorer.ts
@@ -5,6 +5,7 @@ import {
     IItemScore,
     prepareQuery,
     scoreItem,
+    ScorerCache,
 } from "./QuickOpenScorer"
 import { nativeSep } from "./Utilities"
 
@@ -24,7 +25,12 @@ class OniAccessor implements IItemAccessor<any> {
     }
 }
 
-export function scoreItemOni(resultObject: any, searchString: string, fuzzy: boolean): IItemScore {
+export function scoreItemOni(
+    resultObject: any,
+    searchString: string,
+    fuzzy: boolean,
+    cache: ScorerCache,
+): IItemScore {
     if (!searchString) {
         return NO_ITEM_SCORE
     }
@@ -37,14 +43,15 @@ export function scoreItemOni(resultObject: any, searchString: string, fuzzy: boo
 
     const accessor = new OniAccessor()
 
-    return scoreItem(resultObject, query, fuzzy, accessor)
+    return scoreItem(resultObject, query, fuzzy, accessor, cache)
 }
 
 export function compareItemsByScoreOni(
     resultObjectA: any,
     resultObjectB: any,
     searchString: string,
     fuzzy: boolean,
+    cache: ScorerCache,
 ): number {
     if (!searchString) {
         return 0
@@ -58,7 +65,7 @@ export function compareItemsByScoreOni(
 
     const accessor = new OniAccessor()
 
-    return compareItemsByScore(resultObjectA, resultObjectB, query, fuzzy, accessor)
+    return compareItemsByScore(resultObjectA, resultObjectB, query, fuzzy, accessor, cache)
 }
 
 export const getHighlightsFromResult = (result: IMatch[]): number[] => {

diff --git a/browser/src/Services/QuickOpen/Scorer/QuickOpenScorer.ts b/browser/src/Services/QuickOpen/Scorer/QuickOpenScorer.ts
@@ -343,7 +343,7 @@ export function scoreItem<T>(
     query: IPreparedQuery,
     fuzzy: boolean,
     accessor: IItemAccessor<T>,
-    // cache: ScorerCache,
+    cache: ScorerCache,
 ): IItemScore {
     if (!item || !query.value) {
         return NO_ITEM_SCORE // we need an item and query to score on at least
@@ -356,20 +356,20 @@ export function scoreItem<T>(
 
     const description = accessor.getItemDescription(item)
 
-    // let cacheHash: string
-    // if (description) {
-    //     cacheHash = `${label}${description}${query.value}${fuzzy}`
-    // } else {
-    //     cacheHash = `${label}${query.value}${fuzzy}`
-    // }
+    let cacheHash: string
+    if (description) {
+        cacheHash = `${label}${description}${query.value}${fuzzy}`
+    } else {
+        cacheHash = `${label}${query.value}${fuzzy}`
+    }
 
-    // const cached = cache[cacheHash]
-    // if (cached) {
-    //     return cached
-    // }
+    const cached = cache[cacheHash]
+    if (cached) {
+        return cached
+    }
 
     const itemScore = doScoreItem(label, description, accessor.getItemPath(item), query, fuzzy)
-    // cache[cacheHash] = itemScore
+    cache[cacheHash] = itemScore
 
     return itemScore
 }
@@ -467,11 +467,11 @@ export function compareItemsByScore<T>(
     query: IPreparedQuery,
     fuzzy: boolean,
     accessor: IItemAccessor<T>,
-    // cache: ScorerCache,
+    cache: ScorerCache,
     fallbackComparer = fallbackCompare,
 ): number {
-    const itemScoreA = scoreItem(itemA, query, fuzzy, accessor)
-    const itemScoreB = scoreItem(itemB, query, fuzzy, accessor)
+    const itemScoreA = scoreItem(itemA, query, fuzzy, accessor, cache)
+    const itemScoreB = scoreItem(itemB, query, fuzzy, accessor, cache)
 
     const scoreA = itemScoreA.score
     const scoreB = itemScoreB.score

diff --git a/browser/src/Services/QuickOpen/VSCodeFilter.ts b/browser/src/Services/QuickOpen/VSCodeFilter.ts
@@ -15,6 +15,7 @@ import {
 } from "./Scorer/OniQuickOpenScorer"
 
 import { IMenuOptionWithHighlights, shouldFilterbeCaseSensitive } from "./../Menu"
+import { ScorerCache } from "./Scorer/QuickOpenScorer"
 
 export const vsCodeFilter = (
     options: Oni.Menu.MenuOption[],
@@ -49,7 +50,16 @@ export const vsCodeFilter = (
             ? listOfSearchTerms.slice(1).join("") + listOfSearchTerms[0]
             : listOfSearchTerms[0]
 
-    const filteredOptions = processSearchTerm(vsCodeSearchString, options)
+    // Adds a cache for the scores. This is needed to stop the final score
+    // compare from repeating all the scoring logic again.
+    // Currently, this only persists for the current search, which will speed
+    // up that search only.
+    // TODO: Is it worth instead persisting this cache?
+    // Plus side is repeated searches are fast.
+    // Down side is there will be a lot of rubbish being stored too.
+    const cache: ScorerCache = {}
+
+    const filteredOptions = processSearchTerm(vsCodeSearchString, options, cache)
 
     const ret = filteredOptions.filter(fo => {
         if (fo.score === 0) {
@@ -59,15 +69,16 @@ export const vsCodeFilter = (
         }
     })
 
-    return ret.sort((e1, e2) => compareItemsByScoreOni(e1, e2, vsCodeSearchString, true))
+    return ret.sort((e1, e2) => compareItemsByScoreOni(e1, e2, vsCodeSearchString, true, cache))
 }
 
 export const processSearchTerm = (
     searchString: string,
     options: Oni.Menu.MenuOption[],
+    cache: ScorerCache,
 ): Oni.Menu.IMenuOptionWithHighlights[] => {
     const result: Oni.Menu.IMenuOptionWithHighlights[] = options.map(f => {
-        const itemScore = scoreItemOni(f, searchString, true)
+        const itemScore = scoreItemOni(f, searchString, true, cache)
         const detailHighlights = getHighlightsFromResult(itemScore.descriptionMatch)
         const labelHighlights = getHighlightsFromResult(itemScore.labelMatch)
 

diff --git a/browser/test/Services/QuickOpen/VSCodeFilterTests.ts b/browser/test/Services/QuickOpen/VSCodeFilterTests.ts
@@ -3,17 +3,24 @@
  */
 
 import * as assert from "assert"
+import { ScorerCache } from "../../../src/Services/QuickOpen/Scorer/QuickOpenScorer"
 import { processSearchTerm, vsCodeFilter } from "./../../../src/Services/QuickOpen/VSCodeFilter"
 
 describe("processSearchTerm", () => {
+    let cache: ScorerCache
+
+    beforeEach(() => {
+        cache = {}
+    })
+
     it("Correctly matches word.", async () => {
         const testString = "src"
         const testList = [
             { label: "index.ts", detail: "browser/src" },
             { label: "index.ts", detail: "browser/test" },
         ]
 
-        const result = processSearchTerm(testString, testList)
+        const result = processSearchTerm(testString, testList, cache)
         const filteredResult = result.filter(r => r.score !== 0)
 
         // Remove the score since it can change if we updated the
@@ -39,7 +46,7 @@ describe("processSearchTerm", () => {
             { label: "index.ts", detail: "browser/SRC" },
         ]
 
-        const result = processSearchTerm(testString, testList)
+        const result = processSearchTerm(testString, testList, cache)
 
         // Check the exact case match scores higher
         const lowercase = result.find(r => r.detail === "browser/src")
@@ -57,7 +64,7 @@ describe("processSearchTerm", () => {
             { label: "index.ts", detail: "browser/test" },
         ]
 
-        const result = processSearchTerm(testString, testList)
+        const result = processSearchTerm(testString, testList, cache)
         const filteredResult = result.filter(r => r.score !== 0)
 
         assert.deepEqual(filteredResult, [])