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

open_and_sync_buffers is way too slow for large servers #19

Open
comex opened this issue May 21, 2017 · 7 comments
Open

open_and_sync_buffers is way too slow for large servers #19

comex opened this issue May 21, 2017 · 7 comments

Comments

@comex
Copy link

comex commented May 21, 2017

For every update to user/channel state, this program iterates through every channel on every server, then through every user on the server; calculates their nick, asks weechat whether that nick exists in the channel, and adds it if not.

Even if weechat were sane, this would be rather wasteful: O(n^2), assuming the rate of updates is already O(n) in the number of users. And the wasted work is not just a quick list iteration or something, but at minimum includes the allocation and string formatting involved in calculating the nick. But weechat is not sane: nicklist_nick_exists does a linear search over a linked list of nick entries, so this ends up O(n^3), and slow enough in practice to completely lock up my (admittedly puny) EC2 instance (for a Discord server in the thousands of users).

I know this is a work in progress, and you probably know already that that's an inefficient approach, but I'm filing this in case you didn't know just how inefficient :)

The code also never removes nicks, but I guess you know that already too...

@khyperia
Copy link
Owner

Wow. That is... honestly very impressive, I'm amazed how bad my own code is sometimes :)

The only practical reason that I'm syncing the weechat nicklist is so that autocomplete works reasonably well. As someone who is not myself (and has strange use-cases, being the dev), would you think a reasonable solution to this is to just stop syncing to the weechat userlist, and implement sane autocompletion? (so that @khy would complete @khyperia instead of having to type out khy, tab, move to the front of the nick, add the @, then continue writing). Or do you see a lot of value in having the weechat nicklist be updated?

Also, I had no idea that people other than me and one of my friends are using weechat-discord - I've just been kind of hacking away at it without much regard to usability outside of us two. So my apologies if things are a little sub-par, and thank you for the issue report!

@FredrIQ
Copy link

FredrIQ commented Jul 27, 2017

Something that seems to exacerbate this is that open_and_sync_buffers seems to be called far more than it needs to. I tweaked the nicklist update a bit to 1: clear the entire nicklist, 2: then add nicks. This didn't help, so I made it only iterate through 5 nicknames and then bail out. It ended up kind of working but still lagging badly. So I just made the event listener in event_proc.rs instead of calling open_and_sync_buffers, call nothing. This removed the lag, and allowed me to remove the 5-nick limit, but ends up not getting all the nicknames (but still more than 5). Open_and_sync_buffers is called for a reason, but perhaps some of these calls doesn't need to refresh the nicklist for every single server and channel.

EDIT: Manually adding a sync command and running it after the connection to everything is established creates a 15s lag (lurking on several servers with 2k+ people) and properly populates the nicklist. Still slow, but yeah, clearly only syncing server+channel+nicklist when actually needed is the main contender for improvement here.

@khyperia
Copy link
Owner

khyperia commented Aug 8, 2017

Could someone try out the latest master? I just committed 4fc82c1 which likely solves this issue (and the other linked issues). Maybe. It probably also introduces a fair amount of bugs. (Honestly at this point I just want to rewrite my own discord layer instead of using discord-rs, it was a pained match to begin with and it's not getting any better)

@Aanok
Copy link

Aanok commented Aug 9, 2017

Loading the Discord Developers server (~5k members) takes a couple of seconds. @mentions do not autocomplete, however (regular mentions do).

Also I'm not sure if it's related since I've just installed weecord so it could come from an earlier commit, but backlogs are not getting pulled from servers.

@FredrIQ
Copy link

FredrIQ commented Aug 9, 2017

Latest master creates a large lagspike on the initial initialization (being part of several discords with 2k+ members), but seems to work OK afterwards.

@khyperia
Copy link
Owner

khyperia commented Aug 9, 2017

@mentions do not autocomplete, however (regular mentions do).

Yeah, this is expected :( - haven't had the time to figure out how the heck weechat's completion hooks work. It's hard to fix all these things in side projects that I only have a couple hours per week to work on (and open_and_sync_buffers was pretty high priority).

backlogs are not getting pulled from servers.

Unfortunately also expected. No code for this, as it's kind of complex (as it probably needs to be on a background thread, discord-rs blocks on log loading, which means downloading 50 lines of backlog for a couple dozen channels can take >10 seconds of weechat being completely frozen, and I'm scared to find out how long it takes for folks with hundreds of channels).

Latest master creates a large lagspike on the initial initialization

Is this a regression? I'd be worried if it is, but it's expected for weechat to freeze for a few seconds upon /discord connect - and if it went from "completely locked up forever" to "locked for a couple seconds", that's more than okay with me :)... but yeah, again, it's a crappy consequence of discord-rs doing blocking operations.

@FredrIQ
Copy link

FredrIQ commented Aug 11, 2017

Not a regression, considering that the previous behaviour was to become so slow that it was basically unusable.

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

No branches or pull requests

4 participants