Closed RisingOrange closed 3 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 87.51%. Comparing base (
14d9af6
) to head (3538b11
). Report is 6 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@andrewsanchez Good points!
I changed the client to use one Session
for each thread. The Session
is re-used for requests on that thread.
The performance improvements from using a Session properly will depend on us instantiating the client only once when it is being used for multiple requests.
This is now the case, at least when downloading and uploading media (where it matters most). AnkiHubClient.download_media
takes the list of all media names that need to be downloaded, so everything will be handled by one client instance. The same goes for AnkiHubClient.upload_media
.
This pull request was deployed and Sentry observed the following issues:
1322529746.ankihub_client.ankihub_client in _se...
View Issue1322529746.ankihub_client.ankihub_client in _se...
View Issue1322529746.ankihub_client.ankihub_client in _se...
View Issue1322529746.ankihub_client.ankihub_client in _se...
View Issue1322529746.ankihub_client.ankihub_client in _se...
View IssueDid you find this useful? React with a 👍 or 👎
Some users are experiencing Anki being laggy when the AnkiHub media sync is in progress.
This PR addresses the problem by:
requests.Session
objectsmax_workers
argument of theThreadPoolExecutor
instances that download and upload mediaRelated issues
https://ankihub.atlassian.net/jira/software/c/projects/BUILD/boards/1?selectedIssue=BUILD-439 https://theanking.slack.com/archives/C03S5DH9HH9/p1717080091231869
Proposed changes
requests.Session
thread-locallymax_workers
value: https://github.com/ankipalace/ankihub_addon/blob/bb1b2c9cc55a7aa06f14bfa25682f60a51e96788/ankihub/ankihub_client/ankihub_client.py#L115-L118 This reduces the number ofmax_workers
by 3 for most devices when compared to the default setting. The advantage of this is that the amount of workers is based on the device and the reduction will be relatively large on devices with low cpu counts, but relatively small on devices with high cpu counts.