mirror of
https://github.com/vector-im/element-call.git
synced 2026-02-08 04:19:11 +00:00
Cache calls to removeHiddenChars() to fix performance bottleneck in Safari (#3072)
* 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 {
|
||||
removeDirectionOverrideChars,
|
||||
removeHiddenChars,
|
||||
removeHiddenChars as removeHiddenCharsUncached,
|
||||
} from "matrix-js-sdk/src/utils";
|
||||
|
||||
import type { Room } from "matrix-js-sdk/src/matrix";
|
||||
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
|
||||
export function shouldDisambiguate(
|
||||
member: { rawDisplayName?: string; userId: string },
|
||||
|
||||
Reference in New Issue
Block a user