Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Don't reset cached room list values when they are falsey #2413

Merged
merged 1 commit into from
Jan 8, 2019

Conversation

turt2live
Copy link
Member

unread and unread-muted store booleans in the cache, and can easily be false. Without this patch, both of those cached types would be cleared from the object where a later call to getRoomState would try and re-populate them. getRoomState is supposed to use the cache where possible to avoid making the more expensive calls required to calculate those booleans.

On my account in a test environment, this brings the generateRoomLists execution time down from ~250ms to just ~30ms.

This still does not solve the whole issue, but should solve the more common case of performance woes for people.

Should make the app more bearable (see element-hq/element-web#8033)

`unread` and `unread-muted` store booleans in the cache, and can easily be `false`. Without this patch, both of those cached types would be cleared from the object where a later call to `getRoomState` would try and re-populate them. `getRoomState` is supposed to use the cache where possible to avoid making the more expensive calls required to calculate those booleans.

On my account in a test environment, this brings the `generateRoomLists` execution time down from ~250ms to just ~30ms. 

This still does not solve the whole issue, but should solve the more common case of performance woes for people.
@turt2live turt2live requested a review from a team January 8, 2019 00:48
@ara4n
Copy link
Member

ara4n commented Jan 8, 2019

this looks plausible. isn't it more idiomatic to say if (value !== undefined) rather than use typeof though? or am i missing some magic?

@turt2live
Copy link
Member Author

For some reason I did a typeof check in the surrounding code and wanted to keep it consistent.

@turt2live
Copy link
Member Author

also there's a 90% chance I cut all this out later in the week.

@ara4n
Copy link
Member

ara4n commented Jan 8, 2019

ok, lgtm (although would be interested to know what it's being replaced with)

@turt2live turt2live merged commit 58cff9f into experimental Jan 8, 2019
@turt2live turt2live deleted the travis/room-list-go-faster-please branch January 8, 2019 01:01
@turt2live
Copy link
Member Author

It'll be replaced with something that doesn't calculate the room list from scratch every time the client sees an event.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants