-
Notifications
You must be signed in to change notification settings - Fork 271
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
Performance of Room.members
/get_member
in large rooms is bad
#1979
Comments
Just to record what was said on Matrix about this: The fix here will be to provide an API that batches the accesses to the store. Kévin offered to take a stab at it :) |
Should we assign the issue to @zecakeh then? |
We can only assign people who don't have write access if they have commented on the issue, AFAIK ^^ |
@jplatte would this also improve performance for cases where the client needs to fetch all users in the room? The EX apps have two uses cases:
We need both of these to be fast which they're currently not and I'm not sure if paginating things into badges would actually change that? |
For 1., do you count out members that are left or banned? If you only want the count of invited and joined members they are sent by the server during sync and should be available in the room info. It should even be valid before all the members are fetched from the server. I don't know if it is exposed in the bindings though. Even if you want to use the local list of members, there could be methods that are even more efficient just to get the count from the store. |
I think that would be sufficient, yes.
@csmith could you check if we could easily access the count from there? That'll at least improve performance on point 1 then. |
For reference, the methods I'm talking about for the data received via sync are |
Yeah, we can easily access those. Not sure why we're not using them already! I've opened element-hq/element-x-android#505 to do that (I'll get to it today or tomorrow most likely). That'll speed up showing the count on the details screen, but obviously it just moves the delay to a different place if the user actually wants to see the member list :) |
Indeed. We should still use this issue to discuss / fix the slow load of the overall member list. |
Room.members
/get_member
in large channels is badRoom.members
/get_member
in large rooms is bad
@Hywan: on Android most of the time spent in this is in a high end device is (for ~65k members):
Actually retrieving the Rust references from the JVM code is super fast, just a few ms, so I'm sorry for blaming this part when I mentioned this issue earlier. On a mid/low end one you can multiply the time in steps 2 and 3 since performance is quite worse. We can try to address the mapping times by using the actual Rust objects and trying to destroy them when needed (it's easier to have leaks this way though), but maybe having some sort of pagination for loading the members would be best? |
Maybe #2523 will improve things, but… we need to revamp the entire room member list design. I've few faith that #2523 may solve anything relevant for you now, because you're waiting to load everything before showing the list and the search input. What I'm going to do is to restart from scratch this room member list. It's gonna be an The idea is that you'll just have a It needs much more time and investment than expected. |
Some more info on this: I was trying to improve the performance of mapping the room members to our Kotlin classes and I think I found some weird stuff in Rust:
So my takes here are:
|
If the two calls were following each other and there were no gappy sync in this room, then the members are up to date, and the SDK won't reissue a query for the room members. It's likely what you observed here, explaining why the request never got sent. |
The app was restarted between each try, |
I worked on this today to improve the member list performance for Matrix HQ. With a few other improvements like replacing parallelMap with map and increasing the batch size, the Matrix HQ member list now loads ~1m32s the first time (~60s waiting for Matrix HQ, + ~30s for ruma to parse all member events). After that, it always loads in ~2s (even after restarting the app). You can find my work in the timo/fastmembers branch on both the sdk and Element Android. I tried this in an emulator, but @jmartinesp could reproduce it on a 4 year old phone |
What we would also like to have is to expose the member list as an So, ideally, we should load the members in an async task, and in parallel subscribe to the Reducing the FFI boundary crosses is a good thing too, and it's addition to the plan I've explained. |
On Element Android X and Element iOS X we've noticed that it takes a significant amount of time to get the list of members in a room. Eyeballing the network requests and UI, for a room with ~10k members there's an approximately 10-12 second delay between the HTTP request to get members returning, and the UI actually showing anything.
I initially thought this was due to the volume of FFI calls we were making, but I've done some hacky tracing and it seems like all the time is spent in
Room.members
. For the room with 10k members, it takes 30-50ms to get the userIDs for all members, then around 10 seconds in total to callget_member
for each of those users.Inside
get_member
nothing in particular stands out:store.get_member_event
takes 2.5s in aggregatestore.get_presence_event
takes 1.7sstore.get_profile
takes 1.9sstore.get_users_with_display_name
(for the display name ambiguity check) takes 2.0sThere are a couple more bits that I didn't explicitly time, but based on the total time taken I'd imagine they are similar. (Someone with an actual profiler set up may be able to provide more insight here).
Steps to reproduce:
Room.members
The text was updated successfully, but these errors were encountered: