Rewrite the room list store #4253

turt2live · 2020-03-20T20:46:27Z

Fixes element-hq/element-web#11743 (this PR has the core algorithm described in the issue)

Includes & requires #4593 (dispatcher types) (merged)
Requires element-hq/element-web#13675
Requires element-hq/element-desktop#87

Making this behave correctly is left for element-hq/element-web#13635

To manage expectations: This is not as great as it sounds. The room list is in fact rewritten, but there are a whole bunch of missing features that people will have grown to expect. For example, the current room isn't highlighted, filtering doesn't work, and the LLP does nothing against this room list. This is intentional - the diff for the algorithm is large enough as is, so there's no real benefit to having someone review a million lines of feature implementation alongside computing science theory.

Many of the components have half-baked implementations, such as badges: badges show up on initial render but there's TODO comments to update them dynamically. This is due to an arbitrary line drawn in the sand for this PR: the idea was to ensure that the component structure would work to write features, but not necessarily burden the reviewer with trying to figure out how a feature works.

When reviewing, please put more attention on the algorithm and less so the components. The components are intentionally skeletons.

This also doesn't have tests. It's a proof of concept with the absolute bare minimum UI to be considered potentially useful in performance metrics. Tests will come in a later PR when the implementation stabilizes. For now you'll have to trust me that it works :D

The documentation for how this works is in the code. Normally it would be here in this description, likely in place of these exact words, however due to the complexity involved it seemed best to dump the docs directly in the code, making the code worse.

With that being said, here's some implementation notes: This new PR bypasses trying to make the existing room list store and related stack go faster, and instead builds a whole new stack from the ground up. This is taking the form of almost-MVVM with async stores (invented in this PR) where the components get told to render an object of things and to figure out the rest. The most notable difference of this is in the room tiles: previously the component was required to be told what the badge counts and such were, and the new room tile just figures it out. It's not a perfect implementation though - there's some cleanup work still to be done.

For future adventurers: the algorithms are fundamentally different from each other at a technical level with very little reusable code, so they've been split into two kinds: Sorting and List algorithms. The sort algorithms largely define the ordering for a sublist/tag, and the list algorithm defines what to do with the rooms as a higher order sort.

At one point this had all the tagging logic in the RoomListStore2 class, but that was incredibly slow (>500ms) so that was scrapped and the algorithm classes were starting to take shape. This in theory should give a bit of a boost and not cause headaches when trying to read thousands of lines of code.

Targets to beat or match:

300ms initial generation
100ms incremental update

This PR's metrics haven't been measured because the account I would normally use is on a homeserver that can barely handle sending a read receipt right now. It's expected to be at least on par with the numbers above based on limited small-case testing.

bwindels · 2020-03-23T17:23:04Z

Please include the fix for filtering canonical and alternative aliases from #4260

src/stores/AsyncStore.ts

turt2live · 2020-05-14T03:23:11Z

Note to self: this needs to deal with the massive conflict that is #4593

This is to identify how bad of a state we're in to start with.

This does a number of things (sorry): * Estimates the type changes needed to the dispatcher (later to be replaced by #4593) * Sets up the stack for a whole new room list store, and later components for usage. * Create a proxy class to ensure the app still functions as expected when the various stores are enabled/disabled * Demonstrates a possible structure for algorithms

This is to get around the problem of a slow dispatch loop. Instead of slowing the whole app down to deal with room lists, we'll just raise events to say we're ready. Based upon the EventEmitter class.

This is the fruits of about 3 attempts to write code that works. None of those attempts are here, but how edition 4 could work is at least documented now.

Maybe we can speed up the algorithm if we know why we're doing the update.

Instead putting the tag handling in the Algorithm class

Sorting and ordering has now been split apart. The ImportanceAlgorithm also finally makes use of the sorting. So far metrics look okay at 3ms for a simple account, though this could potentially get worse due to the multiple loops involved (one for tags, one for categories, one for ordering). We might be able to feed a whole list of rooms into the thing and have it regenerate the lists on demand.

This is largely meant to prove the algorithm works and nothing more.

A possible approach to handling the various triggers for recategorizing rooms.

See comments within for details on what this means.

For now at least. We shouldn't encounter this case until we get around to adding support for newly-joined rooms.

aka: implement the algorithms.

turt2live · 2020-05-14T20:17:51Z

Please include the fix for filtering canonical and alternative aliases from #4260

This is slated for element-hq/element-web#13635 (already recorded)

This reverts commit 82b55ff.

src/actions/GroupActions.ts

ara4n · 2020-05-14T22:01:28Z

src/actions/RoomListActions.ts

+        let metaData = null;
+
+        // Is the tag ordered manually?
+        if (newTag && !newTag.match(/^(m\.lowpriority|im\.vector\.fake\.(invite|recent|direct|archived))$/)) {


not sure that JS will cache the regexp properly - i'd play it safe and precompile it.

This is the same as what was there before, and for the old room list store - I suspect the whole file will need replacing eventually with the new room list work.

ara4n · 2020-05-14T23:43:16Z

src/components/views/rooms/RoomTile2.tsx

+ */
+
+interface IProps {
+    room: Room;


I'm probably being terribly thick, but: how does RoomTile2.tsx know when the fields of room changes, such that it should go re-render itself?

currently it doesn't, which makes it pretty useless as a component. This is why it's a skeleton :p

foldleft · 2020-05-15T15:15:22Z

src/stores/AsyncStore.ts

+     * @param {T|*} newState The state to update in the store using Object.assign()
+     */
+    protected async updateState(newState: T | Object) {
+        await this.lock.acquireAsync();


I'm confused about why we need this –– there's no sign of concurrency?

The concurrency comes from the usage, not this class. It's entirely possible that 2 functions call setState() at the same time, leading to a very confused state. By locking the state first, we hopefully end up with a more linear sequence of events.

The room list store is still somewhat vulnerable to concurrent events and might need locks as time progresses too.

I assume you don't mean literally "at the same time", since there's no multi-threading happening... So this is about path A calling updateState and then path B calling updateState before the result of A's change has propagated and folded into path B?

yes, sorry. Not exactly at the same time but two concurrent calls being called over top of each other (because multithreading is just pausing code in different branches).

The nature of the async store kinda makes it required as we'd otherwise run into issues eventually.

jryans

Overall, this looks great! 😁

The main open question for me is around the locking, which so far I don't quite understand... but I assume there must be some reason or else you wouldn't have added it, so I assume we can go back and forth on that without blocking the whole review on it.

jryans · 2020-05-21T15:14:11Z

src/components/views/rooms/RoomList2.tsx

@@ -0,0 +1,247 @@
+/*
+Copyright 2015, 2016 OpenMarket Ltd


How much is copied vs. new? Maybe remove the old copyright lines if basically all new...?

It's about 80/20 copied.

src/components/views/rooms/RoomList2.tsx

jryans · 2020-05-21T15:31:26Z

src/components/views/rooms/RoomSublist2.tsx

@@ -0,0 +1,227 @@
+/*
+Copyright 2015, 2016 OpenMarket Ltd


Similarly unsure if these dates make sense.

also about 80% copied.

src/components/views/rooms/RoomSublist2.tsx

jryans · 2020-05-21T15:32:43Z

src/components/views/rooms/RoomTile2.tsx

@@ -0,0 +1,220 @@
+/*
+Copyright 2015, 2016 OpenMarket Ltd


Same as with other files.

I think this one is only 60-70% copied

jryans · 2020-05-21T16:28:47Z

src/stores/room-list/README.md

@@ -0,0 +1,125 @@
+# Room list sorting
+
+It's so complicated it needs its own README.


Thanks for writing all of this down! 🤩

src/stores/room-list/README.md

jryans · 2020-05-21T16:31:09Z

src/stores/room-list/RoomListStore2.ts

+                const receiptUsers = Object.keys(payload.event.getContent()[eventId]['m.read'] || {});
+                if (receiptUsers.includes(myUserId)) {
+                    // TODO: Update room now that it's been read
+                    console.log(payload);


Should the be removed for landing here and in the rest of the branches...?

It'll get removed when the associated TODO is dealt with. It doesn't spam anything unless you opt in to this horrible excuse of a room list.

src/stores/room-list/algorithms/list_ordering/Algorithm.ts

src/stores/room-list/algorithms/tag_sorting/AlphabeticAlgorithm.ts

turt2live · 2020-05-21T17:58:33Z

Thanks @jryans for taking a look 😄

The async locking stuff can definitely be discussed out of band (or here, or on another PR, or somewhere) - we have a little while before this is even remotely close to primetime.

turt2live mentioned this pull request Mar 20, 2020

Possible (minor) speed/stability optimization to room sorting algorithm element-hq/element-web#11743

Closed

turt2live force-pushed the travis/room-list-2 branch 3 times, most recently from f304b82 to 32ba47b Compare April 27, 2020 15:50

ara4n reviewed Apr 28, 2020

View reviewed changes

src/stores/AsyncStore.ts Outdated Show resolved Hide resolved

turt2live force-pushed the travis/room-list-2 branch 3 times, most recently from b8edbb4 to d461643 Compare April 30, 2020 17:54

turt2live force-pushed the travis/room-list-2 branch 4 times, most recently from ec4fbdc to 3a5e640 Compare May 12, 2020 03:09

turt2live mentioned this pull request May 14, 2020

Convert the dispatcher to TypeScript #4593

Merged

turt2live added 15 commits May 14, 2020 12:41

Add temporary timing functions to old RoomListStore

82b55ff

This is to identify how bad of a state we're in to start with.

Invent an AsyncStore and use it for room lists

861268d

This is to get around the problem of a slow dispatch loop. Instead of slowing the whole app down to deal with room lists, we'll just raise events to say we're ready. Based upon the EventEmitter class.

Categorize rooms by effective membership

becadde

Possible framework for a proof of concept

00d400b

This is the fruits of about 3 attempts to write code that works. None of those attempts are here, but how edition 4 could work is at least documented now.

Add another thought

9c04226

Maybe we can speed up the algorithm if we know why we're doing the update.

Remove the need for a tag manager

e7fffee

Instead putting the tag handling in the Algorithm class

Handle DMs

ecf8090

Simple rendering of the room list for visual aid

09b7f39

This is largely meant to prove the algorithm works and nothing more.

Early handling of dispatched events

5dda7f0

A possible approach to handling the various triggers for recategorizing rooms.

Make component index happy

ea34bb3

Work out the new category index for each room update

e1fab9a

See comments within for details on what this means.

Make missing rooms throw instead

4a0d14e

For now at least. We shouldn't encounter this case until we get around to adding support for newly-joined rooms.

Handle event decryption too

e88788f

turt2live added 4 commits May 14, 2020 13:40

Merge branch 'travis/dispatcher-types' into travis/room-list-2

2b4c19e

Use the new TS dispatcher

6cb1efc

Replace ChaoticAlgorithm for tag sorting with deterministic behaviour

91a997d

aka: implement the algorithms.

Replace ChaoticAlgorithm with NaturalAlgorithm for list behaviour

b7ba9b3

turt2live force-pushed the travis/room-list-2 branch from 3a5e640 to b7ba9b3 Compare May 14, 2020 20:16

turt2live added 2 commits May 14, 2020 14:20

Update AsyncStore's docs to be slightly more clear

5cfe29d

Revert "Add temporary timing functions to old RoomListStore"

21e4713

This reverts commit 82b55ff.

turt2live mentioned this pull request May 14, 2020

Add docs and labs flag for new room list implementation element-hq/element-web#13675

Merged

turt2live marked this pull request as ready for review May 14, 2020 20:39

turt2live requested a review from a team May 14, 2020 20:39

turt2live changed the title ~~Rewrite the room list~~ Rewrite the room list store May 14, 2020

ara4n reviewed May 14, 2020

View reviewed changes

src/actions/GroupActions.ts Outdated Show resolved Hide resolved

ara4n reviewed May 14, 2020

View reviewed changes

turt2live requested a review from a team May 14, 2020 22:28

ara4n reviewed May 14, 2020

View reviewed changes

foldleft reviewed May 15, 2020

View reviewed changes

Merge branch 'develop' into travis/room-list-2

38920e7

turt2live requested a review from a team May 19, 2020 05:30

jryans requested review from jryans and removed request for a team May 21, 2020 15:18

jryans approved these changes May 21, 2020

View reviewed changes

turt2live added 3 commits May 21, 2020 11:53

Fix comment style to be less bothersome

559dd98

Which component? The room list!

a11985f

Hyphenize algorithm directories

e3c0b47

turt2live merged commit 7ff850d into develop May 21, 2020

turt2live deleted the travis/room-list-2 branch May 21, 2020 19:30

t3chguy mentioned this pull request May 22, 2020

Keyboard shortcuts aren't working in the latest Riot Desktop Nightly. element-hq/element-web#13763

Closed

turt2live mentioned this pull request Jul 10, 2020

New room list: large account performance element-hq/element-web#14035

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite the room list store #4253

Rewrite the room list store #4253

turt2live commented Mar 20, 2020 •

edited

Loading

bwindels commented Mar 23, 2020

turt2live commented May 14, 2020

turt2live commented May 14, 2020

ara4n May 14, 2020

turt2live May 14, 2020

ara4n May 14, 2020

turt2live May 15, 2020

foldleft May 15, 2020

turt2live May 15, 2020

jryans May 21, 2020

turt2live May 21, 2020

jryans left a comment

jryans May 21, 2020

turt2live May 21, 2020

jryans May 21, 2020

turt2live May 21, 2020

jryans May 21, 2020

turt2live May 21, 2020

jryans May 21, 2020

jryans May 21, 2020

turt2live May 21, 2020

turt2live commented May 21, 2020

		@@ -0,0 +1,125 @@
		# Room list sorting

		It's so complicated it needs its own README.

Rewrite the room list store #4253

Rewrite the room list store #4253

Conversation

turt2live commented Mar 20, 2020 • edited Loading

bwindels commented Mar 23, 2020

turt2live commented May 14, 2020

turt2live commented May 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jryans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turt2live commented May 21, 2020

turt2live commented Mar 20, 2020 •

edited

Loading