From 6f63ff1711664154359bb1b998a80f4274569468 Mon Sep 17 00:00:00 2001
From: David Baker <dbkr@users.noreply.github.com>
Date: Wed, 17 Jul 2024 15:18:46 +0100
Subject: [PATCH] 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: https://github.com/matrix-org/matrix-react-sdk/pull/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
---
 spec/unit/room.spec.ts | 16 ++++++++++++++++
 src/models/room.ts     |  5 +++--
 src/utils.ts           | 10 ----------
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/spec/unit/room.spec.ts b/spec/unit/room.spec.ts
index 542bbcb2536..37b5c15048d 100644
--- a/spec/unit/room.spec.ts
+++ b/spec/unit/room.spec.ts
@@ -1394,6 +1394,22 @@ describe("Room", function () {
                 expect(name).toEqual(userB);
             });
         });
+
+        it("recalculates in acceptable time without heroes", function () {
+            for (let i = 0; i < 5000; i++) {
+                addMember(`@person${i}:bar`, KnownMembership.Join, { name: `Person ${i % 20} ${i % 10} ${i % 3}` });
+            }
+
+            // This isn't a real performance test and has plenty of headroom because github
+            // runners don't offer any kind of speed consistency guarantee, but this should at
+            // least assert that the perf doesn't suddenly become n^2.
+            const start = performance.now();
+            for (let i = 0; i < 50; i++) {
+                room.recalculate();
+            }
+            const duration = performance.now() - start;
+            expect(duration).toBeLessThan(200);
+        });
     });
 
     describe("receipts", function () {
diff --git a/src/models/room.ts b/src/models/room.ts
index ab5eb7bfd9c..0310b88fe34 100644
--- a/src/models/room.ts
+++ b/src/models/room.ts
@@ -24,7 +24,7 @@ import {
 } from "./event-timeline-set";
 import { Direction, EventTimeline } from "./event-timeline";
 import { getHttpUriForMxc } from "../content-repo";
-import { compare, removeElement } from "../utils";
+import { removeElement } from "../utils";
 import { normalize, noUnsafeEventProps } from "../utils";
 import { IEvent, IThreadBundledRelationship, MatrixEvent, MatrixEventEvent, MatrixEventHandlerMap } from "./event";
 import { EventStatus } from "./event-status";
@@ -3451,7 +3451,8 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
                 return true;
             });
             // make sure members have stable order
-            otherMembers.sort((a, b) => compare(a.userId, b.userId));
+            const collator = new Intl.Collator();
+            otherMembers.sort((a, b) => collator.compare(a.userId, b.userId));
             // only 5 first members, immitate summaryHeroes
             otherMembers = otherMembers.slice(0, 5);
             otherNames = otherMembers.map((m) => m.name);
diff --git a/src/utils.ts b/src/utils.ts
index 1d4505f4bb4..0451307d1d1 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -649,16 +649,6 @@ export function lexicographicCompare(a: string, b: string): number {
     }
 }
 
-const collator = new Intl.Collator();
-/**
- * Performant language-sensitive string comparison
- * @param a - the first string to compare
- * @param b - the second string to compare
- */
-export function compare(a: string, b: string): number {
-    return collator.compare(a, b);
-}
-
 /**
  * This function is similar to Object.assign() but it assigns recursively and
  * allows you to ignore nullish values from the source