Skip to content

Commit 6f63ff1

Browse files
authored
Remove the compare function from utils (#4315)
* Remove the compare function from utils and change the one use of it to just intantiate a collator and use it. This was marked as internal module so this shouldn't be a breaking change. Of course, react-sdk was using it. Requires: matrix-org/matrix-react-sdk#12782 * Add simple not-a-perf-test test * recalculate repeatedly otherwise we aren't testing anything different * Use fewer members as it was making the test take a bit too long
1 parent 30a2681 commit 6f63ff1

File tree

3 files changed

+19
-12
lines changed

3 files changed

+19
-12
lines changed

spec/unit/room.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1394,6 +1394,22 @@ describe("Room", function () {
13941394
expect(name).toEqual(userB);
13951395
});
13961396
});
1397+
1398+
it("recalculates in acceptable time without heroes", function () {
1399+
for (let i = 0; i < 5000; i++) {
1400+
addMember(`@person${i}:bar`, KnownMembership.Join, { name: `Person ${i % 20} ${i % 10} ${i % 3}` });
1401+
}
1402+
1403+
// This isn't a real performance test and has plenty of headroom because github
1404+
// runners don't offer any kind of speed consistency guarantee, but this should at
1405+
// least assert that the perf doesn't suddenly become n^2.
1406+
const start = performance.now();
1407+
for (let i = 0; i < 50; i++) {
1408+
room.recalculate();
1409+
}
1410+
const duration = performance.now() - start;
1411+
expect(duration).toBeLessThan(200);
1412+
});
13971413
});
13981414

13991415
describe("receipts", function () {

src/models/room.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
} from "./event-timeline-set";
2525
import { Direction, EventTimeline } from "./event-timeline";
2626
import { getHttpUriForMxc } from "../content-repo";
27-
import { compare, removeElement } from "../utils";
27+
import { removeElement } from "../utils";
2828
import { normalize, noUnsafeEventProps } from "../utils";
2929
import { IEvent, IThreadBundledRelationship, MatrixEvent, MatrixEventEvent, MatrixEventHandlerMap } from "./event";
3030
import { EventStatus } from "./event-status";
@@ -3451,7 +3451,8 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
34513451
return true;
34523452
});
34533453
// make sure members have stable order
3454-
otherMembers.sort((a, b) => compare(a.userId, b.userId));
3454+
const collator = new Intl.Collator();
3455+
otherMembers.sort((a, b) => collator.compare(a.userId, b.userId));
34553456
// only 5 first members, immitate summaryHeroes
34563457
otherMembers = otherMembers.slice(0, 5);
34573458
otherNames = otherMembers.map((m) => m.name);

src/utils.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -649,16 +649,6 @@ export function lexicographicCompare(a: string, b: string): number {
649649
}
650650
}
651651

652-
const collator = new Intl.Collator();
653-
/**
654-
* Performant language-sensitive string comparison
655-
* @param a - the first string to compare
656-
* @param b - the second string to compare
657-
*/
658-
export function compare(a: string, b: string): number {
659-
return collator.compare(a, b);
660-
}
661-
662652
/**
663653
* This function is similar to Object.assign() but it assigns recursively and
664654
* allows you to ignore nullish values from the source

0 commit comments

Comments
 (0)