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
This commit is contained in:
40
src/utils/displayname-integration.test.ts
Normal file
40
src/utils/displayname-integration.test.ts
Normal file
@@ -0,0 +1,40 @@
|
|||||||
|
/*
|
||||||
|
Copyright 2025 New Vector Ltd.
|
||||||
|
|
||||||
|
SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
|
||||||
|
Please see LICENSE in the repository root for full details.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { afterEach, beforeAll, describe, expect, test, vi } from "vitest";
|
||||||
|
|
||||||
|
import { shouldDisambiguate } from "./displayname";
|
||||||
|
import { alice } from "./test-fixtures";
|
||||||
|
import { mockMatrixRoom } from "./test";
|
||||||
|
|
||||||
|
// Ideally these tests would be in ./displayname.test.ts but I can't figure out how to
|
||||||
|
// just spy on the removeHiddenChars() function without impacting the other tests.
|
||||||
|
// So, these tests are in this separate test file.
|
||||||
|
vi.mock("matrix-js-sdk/src/utils");
|
||||||
|
|
||||||
|
describe("shouldDisambiguate", () => {
|
||||||
|
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
|
||||||
|
let jsUtils: typeof import("matrix-js-sdk/src/utils");
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
jsUtils = await import("matrix-js-sdk/src/utils");
|
||||||
|
vi.spyOn(jsUtils, "removeHiddenChars").mockImplementation((str) => str);
|
||||||
|
});
|
||||||
|
afterEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("should only call removeHiddenChars once for a single displayname", () => {
|
||||||
|
const room = mockMatrixRoom({});
|
||||||
|
shouldDisambiguate(alice, [], room);
|
||||||
|
expect(jsUtils.removeHiddenChars).toHaveBeenCalledTimes(1);
|
||||||
|
for (let i = 0; i < 10; i++) {
|
||||||
|
shouldDisambiguate(alice, [], room);
|
||||||
|
}
|
||||||
|
expect(jsUtils.removeHiddenChars).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -7,12 +7,36 @@ Please see LICENSE in the repository root for full details.
|
|||||||
|
|
||||||
import {
|
import {
|
||||||
removeDirectionOverrideChars,
|
removeDirectionOverrideChars,
|
||||||
removeHiddenChars,
|
removeHiddenChars as removeHiddenCharsUncached,
|
||||||
} from "matrix-js-sdk/src/utils";
|
} from "matrix-js-sdk/src/utils";
|
||||||
|
|
||||||
import type { Room } from "matrix-js-sdk/src/matrix";
|
import type { Room } from "matrix-js-sdk/src/matrix";
|
||||||
import type { CallMembership } from "matrix-js-sdk/src/matrixrtc";
|
import type { CallMembership } from "matrix-js-sdk/src/matrixrtc";
|
||||||
|
|
||||||
|
// Calling removeHiddenChars() can be slow on Safari, so we cache the results.
|
||||||
|
// To illustrate a simple benchmark:
|
||||||
|
// Chrome: 10,000 calls took 2.599ms
|
||||||
|
// Safari: 10,000 calls took 242ms
|
||||||
|
// See: https://github.com/element-hq/element-call/issues/3065
|
||||||
|
|
||||||
|
const removeHiddenCharsCache = new Map<string, string>();
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Calls removeHiddenCharsUncached and caches the result
|
||||||
|
*/
|
||||||
|
function removeHiddenChars(str: string): string {
|
||||||
|
if (removeHiddenCharsCache.has(str)) {
|
||||||
|
return removeHiddenCharsCache.get(str)!;
|
||||||
|
}
|
||||||
|
const result = removeHiddenCharsUncached(str);
|
||||||
|
// this is naive but should be good enough for our purposes
|
||||||
|
if (removeHiddenCharsCache.size > 500) {
|
||||||
|
removeHiddenCharsCache.clear();
|
||||||
|
}
|
||||||
|
removeHiddenCharsCache.set(str, result);
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
// Borrowed from https://github.com/matrix-org/matrix-js-sdk/blob/f10deb5ef2e8f061ff005af0476034382ea128ca/src/models/room-member.ts#L409
|
// Borrowed from https://github.com/matrix-org/matrix-js-sdk/blob/f10deb5ef2e8f061ff005af0476034382ea128ca/src/models/room-member.ts#L409
|
||||||
export function shouldDisambiguate(
|
export function shouldDisambiguate(
|
||||||
member: { rawDisplayName?: string; userId: string },
|
member: { rawDisplayName?: string; userId: string },
|
||||||
|
|||||||
Reference in New Issue
Block a user