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

Batch checking live status for all channels after startup. #3757

Merged
merged 6 commits into from
May 22, 2022

Conversation

xel86
Copy link
Contributor

@xel86 xel86 commented May 22, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

On startup, each channel currently individually make an api call to update its own live status on roomId change, this behavior will be unchanged.

Instead of having a timer for each individual channel to call to refresh its own live status, we will have a single timer in TwitchIrcServer that will update the live status of every channel at once in a single api call. Currently I have this set on a 30 second interval. This will be updated faster than the current 60 second interval, but since we are only making a single api call it will still result in an overall performance gain.

Similar logic for NotificationController::fetchFakeChannels() mentioned in the connected issue was already done in PR #3442

Closes #3009

Copy link
Collaborator

@zneix zneix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll comment more things later because someone had to, as always, make my changes before me.

src/providers/twitch/TwitchIrcServer.cpp Outdated Show resolved Hide resolved
src/providers/twitch/TwitchIrcServer.cpp Outdated Show resolved Hide resolved
src/providers/twitch/TwitchIrcServer.cpp Outdated Show resolved Hide resolved
@xel86 xel86 requested review from zneix and Mm2PL May 22, 2022 12:18
Copy link
Collaborator

@zneix zneix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in chat, I'm happy with duplicating logic from TwitchAccount.cpp/NotificationController.cpp here. I will try to make a util wrapping all of these, but that's something for another PR though.

Gonna test this locally first.

Copy link
Collaborator

@zneix zneix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we still make a burst of requests upon starting up the application, this is already a very good improvement 👍

Copy link
Collaborator

@Mm2PL Mm2PL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one little typo and merging :)

src/providers/twitch/TwitchIrcServer.cpp Outdated Show resolved Hide resolved
@Mm2PL Mm2PL changed the title Batch refreshing live status for all channels into single api call Batch checking live status for all channels after startup. May 22, 2022
@Mm2PL Mm2PL enabled auto-merge (squash) May 22, 2022 15:32
@Mm2PL Mm2PL merged commit dc34c16 into Chatterino:master May 22, 2022
@xel86 xel86 deleted the feature/batch-refreshing-live-status branch May 22, 2022 15:53
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

Successfully merging this pull request may close these issues.

Loading live status with a single API call
4 participants