Merge pull request #336291 from federicobrancasi/fb/fuzzy-scorer-cache-hash

performance: reduce Quick Open (Cmd+P) latency by ~20%

src/vs/base/common/fuzzyScorer.ts

@@ -6,7 +6,7 @@
 import { CharCode } from './charCode.js';
 import { compareAnything } from './comparers.js';
 import { createMatches as createFuzzyMatches, fuzzyScore, IMatch, isUpper, matchesPrefix } from './filters.js';
-import { hash } from './hash.js';
+import { ARRAY_HASH_SEED, ConstantStringHash, doHash, numberHash, OBJECT_HASH_SEED, stringHash } from './hash.js';
 import { sep } from './path.js';
 import { isLinux, isWindows } from './platform.js';
 import { equalsIgnoreCase } from './strings.js';
@@ -380,17 +380,42 @@ const PATH_IDENTITY_SCORE = 1 << 18;
 const LABEL_PREFIX_SCORE_THRESHOLD = 1 << 17;
 const LABEL_SCORE_THRESHOLD = 1 << 16;
 
-function getCacheHash(label: string, description: string | undefined, allowNonContiguousMatches: boolean, query: IPreparedQuery) {
+const ALLOW_NON_CONTIGUOUS_MATCHES_KEY_HASH = new ConstantStringHash('allowNonContiguousMatches');
+const DESCRIPTION_KEY_HASH = new ConstantStringHash('description');
+const LABEL_KEY_HASH = new ConstantStringHash('label');
+const VALUES_KEY_HASH = new ConstantStringHash('values');
+const EXPECT_CONTIGUOUS_MATCH_KEY_HASH = new ConstantStringHash('expectContiguousMatch');
+const VALUE_KEY_HASH = new ConstantStringHash('value');
+
+/** Reproduces the generic cache-key hash without constructing its nested object shape. */
+function getCacheHash(label: string, description: string | undefined, allowNonContiguousMatches: boolean, query: IPreparedQuery): number {
+	let hashVal = numberHash(OBJECT_HASH_SEED, 0);
+	hashVal = stringHash(query.normalized, hashVal);
+
+	// Preserve objectHash's sorted property order, including the per-query-piece fields.
+	hashVal = numberHash(OBJECT_HASH_SEED, hashVal);
+
+	hashVal = ALLOW_NON_CONTIGUOUS_MATCHES_KEY_HASH.apply(hashVal);
+	hashVal = doHash(allowNonContiguousMatches, hashVal);
+
+	hashVal = DESCRIPTION_KEY_HASH.apply(hashVal);
+	hashVal = doHash(description, hashVal);
+
+	hashVal = LABEL_KEY_HASH.apply(hashVal);
+	hashVal = doHash(label, hashVal);
+
+	hashVal = VALUES_KEY_HASH.apply(hashVal);
+	hashVal = numberHash(ARRAY_HASH_SEED, hashVal);
 	const values = query.values ? query.values : [query];
-	const cacheHash = hash({
-		[query.normalized]: {
-			values: values.map(v => ({ value: v.normalized, expectContiguousMatch: v.expectContiguousMatch })),
-			label,
-			description,
-			allowNonContiguousMatches
-		}
-	});
-	return cacheHash;
+	for (const value of values) {
+		hashVal = numberHash(OBJECT_HASH_SEED, hashVal);
+		hashVal = EXPECT_CONTIGUOUS_MATCH_KEY_HASH.apply(hashVal);
+		hashVal = doHash(value.expectContiguousMatch, hashVal);
+		hashVal = VALUE_KEY_HASH.apply(hashVal);
+		hashVal = doHash(value.normalized, hashVal);
+	}
+
+	return hashVal;
 }
 
 export function scoreItemFuzzy<T>(item: T, query: IPreparedQuery, allowNonContiguousMatches: boolean, accessor: IItemAccessor<T>, cache: FuzzyScorerCache): IItemScore {

src/vs/base/common/hash.ts

@@ -65,13 +65,41 @@ export function isStringInSample(value: string, samplePercentage: number): boole
 	return (stringHash(value, 0) >>> 0) % 100 < samplePercentage;
 }
 
+/** Precomputes the affine transform of {@link stringHash} for a fixed string and int32 accumulators. */
+export class ConstantStringHash {
+
+	private readonly multiplier: number;
+	private readonly addend: number;
+
+	constructor(value: string) {
+		let multiplier = 31; // accounts for the seed step of `stringHash`
+		for (let i = 0; i < value.length; i++) {
+			multiplier = Math.imul(multiplier, 31);
+		}
+
+		this.multiplier = multiplier;
+		this.addend = stringHash(value, 0);
+	}
+
+	/** Equivalent to `stringHash(value, hashVal)` for int32 `hashVal`. */
+	apply(hashVal: number): number {
+		return (Math.imul(hashVal, this.multiplier) + this.addend) | 0;
+	}
+}
+
+/** Seed mixed in by {@link arrayHash} before hashing the array elements. */
+export const ARRAY_HASH_SEED = 104579;
+
+/** Seed mixed in by {@link objectHash} before hashing the object entries. */
+export const OBJECT_HASH_SEED = 181387;
+
 function arrayHash(arr: unknown[], initialHashVal: number): number {
-	initialHashVal = numberHash(104579, initialHashVal);
+	initialHashVal = numberHash(ARRAY_HASH_SEED, initialHashVal);
 	return arr.reduce<number>((hashVal, item) => doHash(item, hashVal), initialHashVal);
 }
 
 function objectHash(obj: object, initialHashVal: number): number {
-	initialHashVal = numberHash(181387, initialHashVal);
+	initialHashVal = numberHash(OBJECT_HASH_SEED, initialHashVal);
 	return Object.keys(obj).sort().reduce((hashVal, key) => {
 		hashVal = stringHash(key, hashVal);
 		return doHash((obj as Record<string, unknown>)[key], hashVal);

src/vs/base/test/common/fuzzyScorer.test.ts

@@ -5,6 +5,7 @@
 
 import assert from 'assert';
 import { compareItemsByFuzzyScore, FuzzyScore, FuzzyScore2, FuzzyScorerCache, IItemAccessor, IItemScore, pieceToQuery, prepareQuery, scoreFuzzy, scoreFuzzy2, scoreItemFuzzy } from '../../common/fuzzyScorer.js';
+import { hash } from '../../common/hash.js';
 import { Schemas } from '../../common/network.js';
 import { basename, dirname, posix, sep, win32 } from '../../common/path.js';
 import { isWindows } from '../../common/platform.js';
@@ -77,6 +78,14 @@ class NullAccessorClass implements IItemAccessor<URI> {
 	}
 }
 
+function resourceAccessorWithDescription(description: string | undefined): IItemAccessor<URI> {
+	return {
+		getItemLabel: resource => ResourceAccessor.getItemLabel(resource),
+		getItemDescription: () => description,
+		getItemPath: resource => ResourceAccessor.getItemPath(resource)
+	};
+}
+
 function _doScore(target: string, query: string, allowNonContiguousMatches?: boolean): FuzzyScore {
 	const preparedQuery = prepareQuery(query);
 
@@ -292,6 +301,92 @@ suite('Fuzzy Scorer', () => {
 		assert.ok(!res2.score);
 	});
 
+	test('scoreItem - cache key matches the hash of the scoring inputs', () => {
+		const resource = URI.file('/xyz/some/path/someFile123.txt');
+
+		function expectedCacheKey(queryValue: string, allowNonContiguousMatches: boolean, description: string | undefined): string {
+			const query = prepareQuery(queryValue);
+			const values = query.values ? query.values : [query];
+
+			return String(hash({
+				[query.normalized]: {
+					values: values.map(value => ({ value: value.normalized, expectContiguousMatch: value.expectContiguousMatch })),
+					label: ResourceAccessor.getItemLabel(resource),
+					description,
+					allowNonContiguousMatches
+				}
+			}));
+		}
+
+		for (const queryValue of ['some', 'xyz sm', 'xyz "sm"', '', 'ünïcödé', 'null\u0000char', 'tab\there', '0', '01', '4294967295', '__proto__', 'constructor', '\uD83D\uDE80', '\uD800', '\uDFFF']) {
+			for (const description of [undefined, '', ResourceAccessor.getItemDescription(resource), '\u0000:[]', '\uD83D\uDE80\uD800']) {
+				for (const allowNonContiguousMatches of [true, false]) {
+					const cache: FuzzyScorerCache = Object.create(null);
+					scoreItem(resource, queryValue, allowNonContiguousMatches, resourceAccessorWithDescription(description), cache);
+
+					assert.deepStrictEqual(
+						Object.keys(cache),
+						queryValue ? [expectedCacheKey(queryValue, allowNonContiguousMatches, description)] : [],
+						`unexpected cache key for ${JSON.stringify({ queryValue, description, allowNonContiguousMatches })}`
+					);
+				}
+			}
+		}
+	});
+
+	test('scoreItem - cache keys separate items, queries and match modes', () => {
+		const cache: FuzzyScorerCache = Object.create(null);
+
+		// same label, different description
+		scoreItem(URI.file('/xyz/some/path/someFile123.txt'), 'some', true, ResourceAccessor, cache);
+		scoreItem(URI.file('/xyz/other/path/someFile123.txt'), 'some', true, ResourceAccessor, cache);
+
+		// same item, different queries (incl. multi piece and quoted/contiguous)
+		const resource = URI.file('/xyz/some/path/someFile123.txt');
+		scoreItem(resource, 'somefile', true, ResourceAccessor, cache);
+		scoreItem(resource, 'xyz sm', true, ResourceAccessor, cache);
+		scoreItem(resource, 'xyz "sm"', true, ResourceAccessor, cache);
+		scoreItem(resource, 'sm xyz', true, ResourceAccessor, cache);
+
+		// same item and query, different match mode
+		scoreItem(resource, 'somefile', false, ResourceAccessor, cache);
+
+		// an item without description
+		scoreItem(URI.file('/someFile123.txt'), 'some', true, resourceAccessorWithDescription(undefined), cache);
+		scoreItem(URI.file('/someFile123.txt'), 'some', true, resourceAccessorWithDescription(''), cache);
+
+		assert.strictEqual(Object.keys(cache).length, 9, 'each distinct scoring input needs its own cache entry');
+	});
+
+	test('scoreItem - repeated scoring hits the cache and preserves the result', () => {
+		const resource = URI.file('/xyz/some/path/someFile123.txt');
+
+		for (const queryValue of ['somefile', 'xyz sm', 'xyz "sm"', 'ünïcödé']) {
+			const cache: FuzzyScorerCache = Object.create(null);
+			const cold = scoreItem(resource, queryValue, true, ResourceAccessor, cache);
+			const warm = scoreItem(resource, queryValue, true, ResourceAccessor, cache);
+
+			assert.strictEqual(Object.keys(cache).length, 1);
+			assert.strictEqual(warm, cold, 'warm scoring should return the cached result');
+		}
+	});
+
+	test('scoreItem - mutating a prepared query is reflected in the cache key', () => {
+		const resource = URI.file('/xyz/some/path/someFile123.txt');
+		const cache: FuzzyScorerCache = Object.create(null);
+
+		const query = prepareQuery('xyz sm');
+		const before = scoreItemFuzzy(resource, query, true, ResourceAccessor, cache);
+		assert.ok(before.score);
+
+		// callers own the prepared query object, so a mutated query must not reuse the previous entry
+		query.values![1] = { ...query.values![1], expectContiguousMatch: true };
+		const after = scoreItemFuzzy(resource, query, true, ResourceAccessor, cache);
+
+		assert.strictEqual(Object.keys(cache).length, 2);
+		assert.deepStrictEqual(after, scoreItemFuzzy(resource, prepareQuery('xyz "sm"'), true, ResourceAccessor, Object.create(null)));
+	});
+
 	test('scoreItem - invalid input', function () {
 
 		let res = scoreItem(null, null!, true, ResourceAccessor);

src/vs/base/test/common/hash.test.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import assert from 'assert';
-import { isStringInSample } from '../../common/hash.js';
+import { ConstantStringHash, isStringInSample, stringHash } from '../../common/hash.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from './utils.js';
 
 suite('isStringInSample', () => {
@@ -30,3 +30,23 @@ suite('isStringInSample', () => {
 		assert.throws(() => isStringInSample('session', 101));
 	});
 });
+
+suite('ConstantStringHash', () => {
+	ensureNoDisposablesAreLeakedInTestSuite();
+
+	test('matches stringHash for int32 accumulators', () => {
+		const values = ['', 'a', 'label', 'description', 'values', 'value', 'expectContiguousMatch', 'allowNonContiguousMatches', 'ünïcödé', '\u0000\u0001', '\uD83D\uDE80', '\uD800', '\uDFFF', 'x'.repeat(200)];
+		const hashVals = [0, 1, -1, 149417, 2147483647, -2147483648, 123456789, ...Array.from({ length: 128 }, (_, index) => Math.imul(index + 1, 2654435761))];
+
+		assert.deepStrictEqual(
+			values.map(value => hashVals.map(hashVal => new ConstantStringHash(value).apply(hashVal))),
+			values.map(value => hashVals.map(hashVal => stringHash(value, hashVal)))
+		);
+	});
+
+	test('can be chained like stringHash', () => {
+		const chained = new ConstantStringHash('b').apply(new ConstantStringHash('a').apply(0));
+
+		assert.strictEqual(chained, stringHash('b', stringHash('a', 0)));
+	});
+});