Skip to content

Commit f8835d1

Browse files
authored
Merge pull request Expensify#82931 from callstack-internal/perf/cache-buildSearchQueryJSON
Add cache to buildSearchQueryJSON to avoid re-parsing identical queries
2 parents 7b6b2cb + 026de96 commit f8835d1

2 files changed

Lines changed: 106 additions & 1 deletion

File tree

src/libs/SearchQueryUtils.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,20 @@ function getRawFilterListFromQuery(rawQuery: SearchQueryString) {
455455
return undefined;
456456
}
457457

458+
// Cache for buildSearchQueryJSON to avoid re-running the PEG parser for identical queries.
459+
// This is a pure function called from 64+ sites — many fire during the same render cycle
460+
// with identical query strings, each running the full parser from scratch.
461+
const buildSearchQueryJSONCache = new Map<string, SearchQueryJSON | undefined>();
462+
const BUILD_SEARCH_QUERY_JSON_CACHE_MAX_SIZE = 50;
463+
const BUILD_SEARCH_QUERY_JSON_CACHE_KEY_SEPARATOR = '\x00'; // Null byte prevents collisions if query/rawQuery contain arbitrary strings
464+
458465
function buildSearchQueryJSON(query: SearchQueryString, rawQuery?: SearchQueryString) {
466+
const cacheKey = rawQuery ? `${query}${BUILD_SEARCH_QUERY_JSON_CACHE_KEY_SEPARATOR}${rawQuery}` : query;
467+
if (buildSearchQueryJSONCache.has(cacheKey)) {
468+
const cached = buildSearchQueryJSONCache.get(cacheKey);
469+
return cached ? {...cached} : cached;
470+
}
471+
459472
try {
460473
const result = parseSearchQuery(query) as SearchQueryJSON;
461474
const flatFilters = getFilters(result);
@@ -487,7 +500,15 @@ function buildSearchQueryJSON(query: SearchQueryString, rawQuery?: SearchQuerySt
487500
result.rawFilterList = getRawFilterListFromQuery(rawQuery);
488501
}
489502

490-
return result;
503+
if (buildSearchQueryJSONCache.size >= BUILD_SEARCH_QUERY_JSON_CACHE_MAX_SIZE) {
504+
const firstKey = buildSearchQueryJSONCache.keys().next().value;
505+
if (firstKey !== undefined) {
506+
buildSearchQueryJSONCache.delete(firstKey);
507+
}
508+
}
509+
buildSearchQueryJSONCache.set(cacheKey, result);
510+
511+
return {...result};
491512
} catch (e) {
492513
console.error(`Error when parsing SearchQuery: "${query}"`, e);
493514
}

tests/unit/Search/SearchQueryUtilsTest.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ const personalDetailsFakeData = {
3030
},
3131
} as Record<string, {accountID: number}>;
3232

33+
jest.mock('@libs/SearchParser/searchParser', () => {
34+
const actual = jest.requireActual<{parse: (...args: unknown[]) => unknown}>('@libs/SearchParser/searchParser');
35+
return {
36+
...actual,
37+
parse: jest.fn(actual.parse),
38+
};
39+
});
40+
3341
jest.mock('@libs/PersonalDetailsUtils', () => {
3442
const actual = jest.requireActual<typeof PersonalDetailsUtils>('@libs/PersonalDetailsUtils');
3543
return {
@@ -1287,4 +1295,80 @@ describe('SearchQueryUtils', () => {
12871295
expect(result).toContain('merchant:Amazon');
12881296
});
12891297
});
1298+
1299+
describe('buildSearchQueryJSON cache', () => {
1300+
test('mutating the returned object does not affect subsequent calls for the same query', () => {
1301+
const query = `type:expense groupBy:category view:bar date:last-month merchant:test${Date.now()}`;
1302+
1303+
const first = buildSearchQueryJSON(query);
1304+
if (first) {
1305+
first.groupBy = undefined;
1306+
first.view = CONST.SEARCH.VIEW.TABLE;
1307+
}
1308+
1309+
const second = buildSearchQueryJSON(query);
1310+
1311+
expect(second?.groupBy).toBe('category');
1312+
expect(second?.view).toBe('bar');
1313+
});
1314+
1315+
test('returns equal result on repeated calls with the same query', () => {
1316+
const query = 'type:expense status:outstanding';
1317+
1318+
const first = buildSearchQueryJSON(query);
1319+
const second = buildSearchQueryJSON(query);
1320+
1321+
expect(first).toEqual(second);
1322+
});
1323+
1324+
test('returns independent results for the same query with different rawQuery values', () => {
1325+
const query = 'type:expense';
1326+
const rawQueryA = 'type:expense status:drafts';
1327+
const rawQueryB = 'type:expense status:paid';
1328+
1329+
const resultA = buildSearchQueryJSON(query, rawQueryA);
1330+
const resultB = buildSearchQueryJSON(query, rawQueryB);
1331+
1332+
expect(resultA?.rawFilterList).not.toEqual(resultB?.rawFilterList);
1333+
});
1334+
1335+
test('does not cache a failed parse result so subsequent calls retry the parser', () => {
1336+
// Force the parser to throw on the first call only, then succeed on the second.
1337+
// Verifies that a failed parse is not stored in the cache.
1338+
const searchParserModule: {parse: jest.Mock} = jest.requireMock('@libs/SearchParser/searchParser');
1339+
const originalImpl = jest.requireActual<{parse: (...args: unknown[]) => unknown}>('@libs/SearchParser/searchParser').parse;
1340+
1341+
const query = `type:expense merchant:cache-err-test${Date.now()}`;
1342+
let callCount = 0;
1343+
searchParserModule.parse.mockImplementation((...args: unknown[]) => {
1344+
callCount++;
1345+
if (callCount === 1) {
1346+
throw new Error('Simulated parser failure');
1347+
}
1348+
return originalImpl(...args);
1349+
});
1350+
1351+
const firstResult = buildSearchQueryJSON(query);
1352+
const secondResult = buildSearchQueryJSON(query);
1353+
1354+
expect(firstResult).toBeUndefined();
1355+
expect(secondResult?.type).toBe('expense');
1356+
1357+
searchParserModule.parse.mockImplementation(originalImpl);
1358+
});
1359+
1360+
test('evicts the oldest entry when the cache exceeds max size', () => {
1361+
// Insert 51 entries (max is 50) to trigger eviction, then verify
1362+
// the evicted entry can still be re-parsed correctly.
1363+
const uniquePrefix = `type:expense merchant:evict${Date.now()}x`;
1364+
for (let i = 0; i < 51; i++) {
1365+
buildSearchQueryJSON(`${uniquePrefix}${i}`);
1366+
}
1367+
1368+
const afterEviction = buildSearchQueryJSON(`${uniquePrefix}0`);
1369+
1370+
expect(afterEviction).toBeDefined();
1371+
expect(afterEviction?.type).toBe('expense');
1372+
});
1373+
});
12901374
});

0 commit comments

Comments
 (0)