Skip to content

Commit 30b5bcd

Browse files
authored
Cache calls to removeHiddenChars() to fix performance bottleneck in Safari (#3066)
* Cache calls to removeHiddenChars() as very slow on Safari Fixes #3065 * Test * Split testing for removeHiddenChars
1 parent b635b00 commit 30b5bcd

File tree

2 files changed

+65
-1
lines changed

2 files changed

+65
-1
lines changed
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
Copyright 2025 New Vector Ltd.
3+
4+
SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
5+
Please see LICENSE in the repository root for full details.
6+
*/
7+
8+
import { afterEach, beforeAll, describe, expect, test, vi } from "vitest";
9+
10+
import { shouldDisambiguate } from "./displayname";
11+
import { alice } from "./test-fixtures";
12+
import { mockMatrixRoom } from "./test";
13+
14+
// Ideally these tests would be in ./displayname.test.ts but I can't figure out how to
15+
// just spy on the removeHiddenChars() function without impacting the other tests.
16+
// So, these tests are in this separate test file.
17+
vi.mock("matrix-js-sdk/src/utils");
18+
19+
describe("shouldDisambiguate", () => {
20+
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
21+
let jsUtils: typeof import("matrix-js-sdk/src/utils");
22+
23+
beforeAll(async () => {
24+
jsUtils = await import("matrix-js-sdk/src/utils");
25+
vi.spyOn(jsUtils, "removeHiddenChars").mockImplementation((str) => str);
26+
});
27+
afterEach(() => {
28+
vi.clearAllMocks();
29+
});
30+
31+
test("should only call removeHiddenChars once for a single displayname", () => {
32+
const room = mockMatrixRoom({});
33+
shouldDisambiguate(alice, [], room);
34+
expect(jsUtils.removeHiddenChars).toHaveBeenCalledTimes(1);
35+
for (let i = 0; i < 10; i++) {
36+
shouldDisambiguate(alice, [], room);
37+
}
38+
expect(jsUtils.removeHiddenChars).toHaveBeenCalledTimes(1);
39+
});
40+
});

src/utils/displayname.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,36 @@ Please see LICENSE in the repository root for full details.
77

88
import {
99
removeDirectionOverrideChars,
10-
removeHiddenChars,
10+
removeHiddenChars as removeHiddenCharsUncached,
1111
} from "matrix-js-sdk/src/utils";
1212

1313
import type { Room } from "matrix-js-sdk/src/matrix";
1414
import type { CallMembership } from "matrix-js-sdk/src/matrixrtc";
1515

16+
// Calling removeHiddenChars() can be slow on Safari, so we cache the results.
17+
// To illustrate a simple benchmark:
18+
// Chrome: 10,000 calls took 2.599ms
19+
// Safari: 10,000 calls took 242ms
20+
// See: https://github.com/element-hq/element-call/issues/3065
21+
22+
const removeHiddenCharsCache = new Map<string, string>();
23+
24+
/**
25+
* Calls removeHiddenCharsUncached and caches the result
26+
*/
27+
function removeHiddenChars(str: string): string {
28+
if (removeHiddenCharsCache.has(str)) {
29+
return removeHiddenCharsCache.get(str)!;
30+
}
31+
const result = removeHiddenCharsUncached(str);
32+
// this is naive but should be good enough for our purposes
33+
if (removeHiddenCharsCache.size > 500) {
34+
removeHiddenCharsCache.clear();
35+
}
36+
removeHiddenCharsCache.set(str, result);
37+
return result;
38+
}
39+
1640
// Borrowed from https://github.com/matrix-org/matrix-js-sdk/blob/f10deb5ef2e8f061ff005af0476034382ea128ca/src/models/room-member.ts#L409
1741
export function shouldDisambiguate(
1842
member: { rawDisplayName?: string; userId: string },

0 commit comments

Comments
 (0)