Use multiple threads for refreshing feeds #7126
Conversation
|
Thanks for working on this. It looks like this breaks the progress notification, which now always displays the entire list of subscriptions instead of showing the ones that still need to be refreshed. Could you have a look at that, please? |
Update the notification when done downloading instead of before since multiple will be running at the end and the notification would show feeds that are already done.
|
Of course, just pushed a change to fix that. |
| refreshFeed(feed, force); | ||
| //Unchecked is used to prevent a race condition where a feed finishes downloading before | ||
| // we're done submitting all of the jobs | ||
| List<Feed> unchecked = new ArrayList<Feed>(toUpdate); |
There was a problem hiding this comment.
Is this copy actually needed? You are only updating the list in a synchronized block anyway. Is this maybe a leftover from an earlier experiment?
There was a problem hiding this comment.
I think it might be, because we are iterating over toUpdate and I guess we shouldn't remove items from a list we are currently iterating over.
There was a problem hiding this comment.
Yeah, that's correct if it were to somehow manage to get to the remove before the for loop was done, we
d get an ConcurrentModificationException so the copy is there to prevent that.
| // we're done submitting all of the jobs | ||
| List<Feed> unchecked = new ArrayList<Feed>(toUpdate); | ||
| updateNotification(unchecked); | ||
| ExecutorService executor = Executors.newFixedThreadPool(5); |
There was a problem hiding this comment.
5 sounds a bit arbitrary. I think the default was 4 when we had parallel refresh before (which got removed after a rewrite of the download code)
|
Thanks! Will be released in 3.5.x |
Description
The feed refreshing uses only a single thread which can take quite some time when many feeds are being refreshed. Do this on a few threads to speed this up.
From my testing this can reduce the time it takes to refresh the feeds from 30 seconds to 11 seconds (Ran with 15 outdated feeds).
Checklist
./gradlew checkstyle spotbugsPlayDebug spotbugsDebug :app:lintPlayDebug