Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazyload findings #7182

Closed
3 of 8 tasks
ara4n opened this issue Aug 17, 2018 · 12 comments
Closed
3 of 8 tasks

Lazyload findings #7182

ara4n opened this issue Aug 17, 2018 · 12 comments

Comments

@ara4n
Copy link
Member

ara4n commented Aug 17, 2018

So, I've just tried turning on LL on my main account in riot-web. So far, cautiously good - but initial feedback is:

  • The initialSync still seemed to take a very long time to calculate on the server. I'm wondering whether in all the PR review we're still pulling all the membership state out of the DB, making that leg of the journey as slow as ever
  • More worryingly, we don't seem to unblock the initialSync and launch the app once the /sync has completed - instead, it sits there calling /members in the background whilst still showing the initial spinner; i think potentially for all my E2E rooms. As a result, it takes as long (longer?) than ever to get access to the app. Surely we should only be spinning up /members in the background?
  • We see a flicker of stale membership state in the membership list until the bg /members has completed on loading a given room. We should presumably suppress the /members list entirely until it's loaded?
  • I tried to switch room to Matrix HQ. But because I'm not in the lazyloaded members (having not spoken there recently) it showed me the Peek view of the room, until the bg /members had completed, at which point it recovered. How should we fix this? Always include the user themselves in the LL memberss? (except this feels a bit redundant?) Or should we flag genuine peeking rooms somehow so we can differentiate them from this scenario?
  • RR positions only seem to update once the bg /members update completes, which causes a nasty pop as both the RRs suddenly click into position and the /members list loads. Instead we should surely be showing the RRs in the right place but just populating up their profiles (if needed) once /members loads?
    • I just saw this again, and this time the RRs suddenly changed position quite long after the memberlist had LL'd (and updated from a handful to the full list). So i don't think it's blocked on just the bg /members completing, but something else.
  • We no longer show a spinner when recovering after losing connectivity (e.g. after being slept). Superficially this looks like the catchup has entirely failed, as the client lets you send messages, but there's no evidence that the client doesn't think it's caught up. This might be the same as the previous "RR suddenly jump" bug? (if RR are the only things to have changed position after having caught up). In practice, though, it's still trying to reconnect. Rageshake. It looks like this:

screen shot 2018-08-17 at 23 34 19

  • If you start using the FilePanel view, it seems that the main timeline can now get stuck in FilePanel view too after a refresh or backgrounding/foregrounding; all my rooms are only showing attachments only currently(!). This looks like it was only happening in the clientside filter, as (luckily) refreshing the page fixed it.
  • The whole app UI freezes badly when the bg /members query returns
@ara4n
Copy link
Member Author

ara4n commented Aug 18, 2018

and for some good news, my v8 heap size has gone down from 1.2GB to 350MB ;)

@ara4n
Copy link
Member Author

ara4n commented Aug 20, 2018

I think the simplest and most correct fix on stopping the "every room appears as a peek until /members load" misbehaviour is to just get the server to LL you your own member whatever.

@ara4n
Copy link
Member Author

ara4n commented Aug 22, 2018

ftr I did the server-side "always lazyload yourself" work too over at matrix-org/synapse@develop...matthew/lazy_load_yourself and matrix-org/sytest@develop...matthew/lazy_load_yourself - but @bwindels is absolutely right that this can be done more elegantly clientside as per matrix-org/matrix-react-sdk#2126

@manuroe
Copy link
Member

manuroe commented Aug 22, 2018

about item 2: it sits there calling /members in the background whilst still showing the initial spinner; i think potentially for all my E2E room

FTR, the initial /sync on mobile SDKs does not block on the completion of their equivalent Crypto.onCryptoEvent(). Its management continues in parallel.

I even wonder if we could use only known members. The matrix-ios-sdk crypto tests are even still happy if I pass an empty list of members on onCryptoEvent.

FTR bis, in case the /members request fails, the iOS code fallbacks to the known members to continue and to avoid to lose the provided key (commit).

FTR ter, I have this test to check e2e in a room where we do not have all members loaded.

@ara4n
Copy link
Member Author

ara4n commented Aug 22, 2018

we only need to know the members in a E2E room before we send a message into it (assuming that the server correctly tells the members of the room that our device exists in the room). So, i think it should be fine to delay /members for E2E rooms until that room is actually loaded (and the user tries to speak in it), and it certainly shouldn't block initial login.

@manuroe
Copy link
Member

manuroe commented Aug 22, 2018

Members are used here to start tracking their devices. I need to check the impact if we do not track devices beforehand anymore.

@bwindels
Copy link
Contributor

@ara4n any hints how I can reproduce the FilePanel? I tried opening the file panel, minimizing the browser, changing tab, making my laptop sleep, going back to the riot tab, couldn't repro so far.

@ara4n
Copy link
Member Author

ara4n commented Aug 23, 2018

unsure. i just kept opening filepanel in various rooms and switching around, and then next time i unslept the app the main timeline was using the filepanel filter. i only saw it once.

@bwindels
Copy link
Contributor

bwindels commented Aug 28, 2018

The RR issue really looks the same as item 6, it's just the catchup sync taking a long time. Investigated a bit but couldn't reproduce them being blocked on /members.

I'd leave it as it is and hope for /sync to get faster?

@ara4n
Copy link
Member Author

ara4n commented Aug 31, 2018

hm. i'm still seeing "you cannot post in this room" in the composer until the bg members have loaded when switching to a room for the first time since an initial sync.

@ara4n
Copy link
Member Author

ara4n commented Aug 31, 2018

I just reproduced the FilePanel problem:

  • I joined a room. Before(?) the LL members synced up, i clicked on the FilePanel icon. I didn't try scrolling the FilePanel.
  • I then started paginating backwards in the main timeline
  • The only things which showed up in the scrollback for the main timeline were filtered files; no normal messages.

In other news, we should split this bug up into separate ones... i only dumped it all in one place due to expedience/laziness whilst on hol.

@bwindels
Copy link
Contributor

bwindels commented Sep 3, 2018

Opened individual tickets for the remaining issues, let's continue the conversation there.

@bwindels bwindels closed this as completed Sep 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants