Chatterino / chatterino2

Chat client for https://twitch.tv
MIT License
2.06k stars 449 forks source link

Loading live status with a single API call #3009

Open Mm2PL opened 3 years ago

Mm2PL commented 3 years ago

Describe your issue

Currently Chatterino loads the live status by using a lot of Helix API calls. It's slow and wastes a lot of API calls. Helix supports querying multiple users by both login and ID at the same time. Batching these would speed up live detection dramatically.

Screenshots

Here is a screenshot I made after applying #2991 to show HTTP requests.

GET https://api.twitch.tv/helix/streams?user_id=ID 200"

OS and Chatterino Version

2991

zneix commented 3 years ago

Such thing would require pagination though I'm pretty sure. Also, why is this a bug? I'd consider it rather an enhancement to make the already working feature better.

Mm2PL commented 3 years ago

The documentation states that you can check up to 100 streams (100 by ID, 100 by login and max result length is 100). It might require pagination, however what sane person is gonna have more than 100 channels open and have their IRC still work.

zneix commented 3 years ago

I had a look deeper into the code and we execute this request in two places:

  1. NotificationController::getFakeTwitchChannelLiveStatus (by login) - for channels that in Live Notifications > Selected Channels that aren't added as splits.
  2. TwitchChannel::refreshLiveStatus (by id) - once on successfully loading channel's id and then every 60 seconds via QTimer.

Pretty sure that while 1. can be easily tweaked to fetch live status periodically for logins (or ids that would be once queried and stored somewhere under NotificationController), but 2. would require making some timer under TwitchIrcServer, invoking some signal for a selected channel once stream status has changed.

I think we could even easily make calls more often than 60 seconds, but given my solution above for 2. I'm not sure how fetching upon loading room's ID would work - that's usually when channels are first loaded on application start or are added manually via Ctrl+T. As a solution to my last sentence, I'm thinking that fetching live status of all channels every 10 seconds in a batch could pretty much solve a case, where adding new channel wouldn't be too much of an issue, as you'd need to wait ~9s at most to get to know that channel is live after adding it. Another solution is to force / speed up another bulk request for fetching live status of channels, but I'm not sure if that wouldn't be a bad idea in case someone would be re-adding channels constantly.

I can take this up, but I'd need some feedback on my proposed solutions, what do you think about my rambling? (cc @pajlada @leon-richardt @mm2pl)

leon-richardt commented 3 years ago

what do you think about my rambling?

Sounds reasonable, I'm not sure whether I completely understand the problem. What would be the issue with also making a bulk-request whenever a new channel is added? The bulk-request timer could then be restarted.

in case someone would be re-adding channels constantly

This would be also the case with the current implementation, correct? If so, I don't think it's necessary to spend additional effort on treating that scenario.

Mm2PL commented 3 years ago

Loading all channels' live status after joining a new one sounds okay enough to me. It's not that expensive, especially compared to what we do now.

zneix commented 2 years ago

Even though this was closed, the GH-3757 does not fully resolve the issue we're facing here - all single channels fetch their live status "one by one" right after startup. Currently it's not possible due to how our TwitchChannel::roomId update logic works, so it has to be postponed for later. Regardless, while it's not a lot, I think it's relevant to batch all the requests as much as possible, so I'll reopen this.