wee-slack / wee-slack

A WeeChat script for Slack.com. Supports threads and reactions, synchronizes read markers, provides typing notification, etc..
MIT License
2.53k stars 226 forks source link

very high CPU usage after e626714 #835

Open dustymabe opened 3 years ago

dustymabe commented 3 years ago

I just upgraded my low-resource weechat client box and notice very high CPU usage and slow performance to the point where just typing takes seconds sometimes. I was previously on v2.6.0 with no issues. I did a bisect to find where the problem was introduced:

$ git bisect bad
e626714a1acdc13fd711869b879e2db7aa75ace8 is the first bad commit
commit e626714a1acdc13fd711869b879e2db7aa75ace8
Author: Trygve Aaberge <trygveaa@gmail.com>
Date:   Sat Feb 20 12:31:58 2021 +0100

    Replace deprecated APIs with conversations API

    Now that conversations.mark works for all tokens, just using the
    conversations API the same way as the old APIs works fine as far as I
    can see.

    Loosing unread_count_display by replacing mpim.open with
    conversations.open and channels.info with conversations.info means that
    mpims and channels will not be marked as unread when wee-slack loads if
    background_load_all_history has been set to false anymore. As far as I
    can see, the only way to check if they have unread messages is by
    loading the history, so there is no way around this.

    Fixes #792

 README.md    | 16 +++++++++-------
 wee_slack.py | 26 +++++++++++++-------------
 2 files changed, 22 insertions(+), 20 deletions(-)

Looks it was introduced with e626714. Can we look into what happened there and get our CPU usage back to a reasonable level?

trygveaa commented 3 years ago

Yeah, I know :(

Do you have thread_messages_in_channel enabled? With the new API we have to do lots of API call to support that, which kills performance.

dustymabe commented 3 years ago

Yeah, I know :(

Oops. Sorry to poor salt in the wound.

Do you have thread_messages_in_channel enabled? With the new API we have to do lots of API call to support that, which kills performance.

Yes, I do have that enabled. It's quite a nice feature.

trygveaa commented 3 years ago

So the thing is that previously we only had to make one request for the history per channel, and we would get the history including threads. Now we have to make one request for the channel history, and additionally one request for each thread in the history we fetch. I don't know a way around this, but I would of course be glad if someone found a better way with the API. The old API doesn't work anymore, so we can't use that.

I also use that feature, so my solution is to basically almost never reload wee-slack, since only the startup is affected.

dustymabe commented 3 years ago

Interesting. Thanks for the context. I was able to bring things back to a more reasonable level by:

/set plugins.var.python.slack.background_load_all_history false

Which prevents trying to load a crazy amount of history up front. I think it was really killing my instance on this low power machine.