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

Accounts panel & UserInfo RCEP rework #140

Merged
merged 7 commits into from
Jul 1, 2024

Conversation

RedFlames
Copy link
Collaborator

@RedFlames RedFlames commented May 30, 2024

  • Changed UserData LoadAll & LoadRegistered to return Dictionary with UID as keys. Few places actually used these, pretty much just in Frontend stuff.
  • Also, keep 'tagged' users in dict in Frontend module in C#.

With both of these, can avoid checking all many thousand BasicUserInfo for uncluttered accounts panel this way.
New RCEP /api/userinfosfiltered takes params:

  • onlyspecial={true|false} to only return the tagged users from cached Dict, and fetch BanInfos and KickHistorys, currently it returns 458 accounts/User infos (optional, default false)
  • forcereload={true|false} to refresh the mentioned dict of tagged users from database (optional, default false)
  • from={first}&count={count} same as in existing /userinfos EP (optional, default 0 - 100)
  • search={search} filter names, ignores case (optional)

Specifically the change to UserData is this:

-        public abstract T[] LoadRegistered<T>() where T : new();
-        public abstract T[] LoadAll<T>() where T : new();
+        public abstract Dictionary<string, T> LoadRegistered<T>() where T : new();
+        public abstract Dictionary<string, T> LoadAll<T>() where T : new();

(plus changes to the implementations of course)

TODO:

  • pagination in panel based on from=X&to=Y endpoint params
  • searching in userinfo via new endpoint
  • maybe individual toggles for tagged users, bans, kicks?

Overview of the issue and timings

image

With the very scientific method of "where do I have to place the scroll bar to see certain ranges of durations" I can see that all the userinfos EP updates that happen on the actual server on sans take on average 9 seconds, +/- 1.
Some take as long as 17 seconds.
(I'm currently also questioning if we should stop making the websocket trigger these automatically entirely?... Since I'm pretty sure it was broken for the longest time anyways.)

cnet_server_userinfos_time_2024-06-22 155416
Running the server locally on my PC, it takes almost half a minute to fetch the entire 7+ MB of userinfos. I'm surprised how bad this is.
PS: Oh yeah and when switching to "cluttered" view on current main build, we know that it just gives the browser a heart attack because it adds 40k entries to the panel list. Including the ~25s it takes about 80 to 90s for my Firefox tab to recover from that ordeal at the moment.

Running with this PR, it defaults to the new endpoint, which will cache "tagged" users in memory:
cnet_server_userinfos_time_taggeds_2024-06-22
This takes about 7 seconds at startup for me, and can also be triggered by the EP on demand, but shouldn't be necessary all that often.
The other categories of "uncluttered"/special users would be kicks and bans, which can already be loaded efficiently since the respective serialized data blobs already contain the UIDs and don't require going through the entire database of BasicUserInfos.

cnet_server_userinfos_time_serverfilter_2024-06-22
This is how long requests take with server-side filtering, where pagination will only fetch slices of 200 accounts at a time. The downside to this is that currently there's no good way to page through a list of accounts sorted by name or anything, since that would take the 10 or 25 second penalty.
Letting SQLite sort them is not viable either because the names and such are stuck in the serialized blobs (we're essentially running more of a MongoDB type document storage than anything even remotely relational)

Or you can use the "in browser" filtering mode, where you HAVE to manually invoke the 10s / 25s full fetch of userinfos once but then you can filter and sort near-instantly in browser.
cnet_server_userinfos_browserfilter_2024-06-22

The tooltips of what the buttons do:
image
The text shown is always what it's currently set to. The "Filter Mode" toggle changes the meaning of the Reload/Refresh button and associated logic:

  • In "Filter Mode: On Server", a panel refresh will always fetch the relevant userinfos from the server, same for e.g. typing in a search term.
  • In "Filter Mode: In Browser" as I said, only pressing the "Reload All" button will fetch the whole 7 MB or however many, it shouldn't happen automatically.

The "Filter" of the third button was previously "unclutter/clutter" with the visibility icon. It is now "Filter: Show All" for the "cluttered" view and "Filter: Kick/Ban/Tag" for "uncluttered".

Again for the same reasons stated as for sorting, the server-side filtering option currently does no calculations for how many total results your search may return. So pagination gets a bit silly.

E.g. for the 458 "filtered" accounts, you get pages 2, 3, ...:
image
Where it simply fetches "from=200&to=400" and so on, and on page 4 and beyond there are no more results to be found.

I don't see this as a huge issue tbh and currently have no plans to fix this unless we overhaul UserData modules tbh 🧌

In "in browser" filter mode you get correct total pages at least but you can still go beyond because idc to fix :3
image

RedFlames added 3 commits May 30, 2024 16:36
…ID as keys. Also, keep 'tagged' users in dict in Frontend module in C#. With both of these, can avoid checking all many thousand BasicUserInfo for uncluttered accounts panel this way. New RCEP /api/userinfosfiltered only works with onlyspecial=true for now, planning to implement search-based filtering otherwise.
@RedFlames RedFlames marked this pull request as ready for review June 22, 2024 15:12
And ClientInfo broke slightly because there's now a 'Registry' namespace in Everest.
…s and kicks would've potentially put duplicate entries in accounts list
@RedFlames RedFlames merged commit afb7841 into 0x0ade:main Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant