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

ReturnYouTubeDislike connection statistics. Show client toast messages on RYD connection errors. High priority background threads. #236

Merged

Conversation

LisoUseInAIKyrios
Copy link
Contributor

I changed the background thread pool so all threads run at max priority.

This change seems to reduce the average time needed to load the video UI, as the RYD vote fetch now gets priority over other background threads (the fetch is blocking the UI, it definitely should get priority).

Currently this thread pool is only used by RYD for the fetch votes calls.

@oSumAtrIX
Copy link
Member

Can you include benchmark results

@LisoUseInAIKyrios
Copy link
Contributor Author

Benchmarking is not precise since it's affected by whatever is going on in the background, how fast YouTube responds for each video, internet connection speed at that moment, etc, etc, etc.

Without patch. Loading 20 random videos:

revanced: ReturnYouTubeDislike: UI thread paused for: 1071ms waiting for vote fetch to complete
revanced: ReturnYouTubeDislike: UI thread paused for: 1221ms waiting for vote fetch to complete
revanced: ReturnYouTubeDislike: UI thread paused for: 661ms waiting for vote fetch to complete
revanced: ReturnYouTubeDislike: UI thread paused for: 61ms waiting for vote fetch to complete

With patch. Loading same 20 random videos:

revanced: ReturnYouTubeDislike: UI thread paused for: 762ms waiting for vote fetch to complete
revanced: ReturnYouTubeDislike: UI thread paused for: 159ms waiting for vote fetch to complete

@oSumAtrIX
Copy link
Member

Unsure if there is any improvement, both benchmarks show similar results.

@LisoUseInAIKyrios
Copy link
Contributor Author

LisoUseInAIKyrios commented Dec 12, 2022

without the patch, the UI waited 4 out of 20 times.

With the patch, it waited 2 out of 20 times.

It does not log when the UI did not wait.

@oSumAtrIX
Copy link
Member

Oh I see, i thought these were samples taken out of the 20.

@LisoUseInAIKyrios
Copy link
Contributor Author

LisoUseInAIKyrios commented Dec 12, 2022

Another way to view:

  • before the patch, the UI was stalled for 3,014ms to load 20 videos
  • after the patch, the UI was stalled for 921ms to load 20 videos.

@LisoUseInAIKyrios LisoUseInAIKyrios marked this pull request as draft December 14, 2022 00:06
@LisoUseInAIKyrios LisoUseInAIKyrios changed the title high priority background threads high priority background threads. ReturnYouTubeDislike statistics. Show client toast messages on RYD connection errors. Dec 16, 2022
@LisoUseInAIKyrios
Copy link
Contributor Author

I added logging to the API network calls, and added user viewable statistics if debugging is enabled:
https://freeimage.host/i/HoDNhKB

Also added a Toast if a connection times out, or if a rate limit is encountered.

@LisoUseInAIKyrios LisoUseInAIKyrios marked this pull request as ready for review December 18, 2022 07:57
@LisoUseInAIKyrios LisoUseInAIKyrios changed the title high priority background threads. ReturnYouTubeDislike statistics. Show client toast messages on RYD connection errors. ReturnYouTubeDislike connection statistics. Show client toast messages on RYD connection errors. High priority background threads. Dec 18, 2022
@oSumAtrIX oSumAtrIX force-pushed the high_priority_background_threads branch from e017af5 to a240821 Compare December 21, 2022 20:48
@oSumAtrIX oSumAtrIX merged commit 693ef08 into ReVanced:main Dec 21, 2022
@oSumAtrIX
Copy link
Member

Thanks!

@github-actions
Copy link
Contributor

🎉 This PR is included in version 0.86.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@LisoUseInAIKyrios LisoUseInAIKyrios deleted the high_priority_background_threads branch December 21, 2022 21:34
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.

None yet

2 participants