Improve UI responsiveness during RSS downloading. Closes #873, #1089, #1235, #5423

[magao]

This is one of a series of pull requests designed to improve the responsiveness of the UI. This pull request addresses #873, #1089, #1235, #5423 (which are all essentially the same issue reported multiple times).

The issues mentioned talk about using QThread, and my initial intention was to move the processing of the RSS articles fully to the thread currently being used for RSS parsing, but I wasn't able to work out how to get it to work. I have taken a different approach in this pull request - process a single article, then fire a timer to process the next article with a 1ms delay. This allows UI and input events to be processed almost immediately instead of the entire UI freezing until the feed is processed.

Note: Qt seems to do something special with a 0ms delay - I'm guessing adding it to the current events to be processed rather than actually going around the event loop - which resulted in a much less responsive UI than using 1ms.

This pull request also improves startup time, since the feeds were being processed before the UI was displayed. With this change qBittorrent is usable within a short period (~20 seconds in the VM on my QNAP) instead of having to wait for the feed processing (which took multiple minutes in my VM).

[thalieht]

Lower case d in m_deferredDownloads and the one above it please.

[zeule]

The issues mentioned talk about using QThread, and my initial intention was to move the processing of the RSS articles fully to the thread currently being used for RSS parsing, but I wasn't able to work out how to get it to work

Why? All the complications with timers and partial processing create quite a depressing impression...

[magao]

My Qt knowledge is very limited (prior to embarking on these changes it was zero), and the information online is pretty contradictory regarding QThread. And looking at issue #1235 there is already a fair amount of confusion on this.

I had an initial attempt at using QThread, and thought I'd implemented it correctly, but it didn't give any improvement and adding some debug logging revealed that everything was still running on the main thread. At which point I came to the conclusion that it would require someone with much better knowledge of QThread than I have to do it correctly, and went with an approach which - while ugly - actually worked.

[magao]

Rebased on top of #6212 and uncrustified.

[zeule]

You may use std::thread (which I would actually prefer over QThread) if that is the problem.

[magao]

I believe it's the interaction of threads, signals and slots that is the problem, and what I couldn't get right. I suspect that using QThread would be a requirement for that. Plus QThread is already in use for the RSS feed parsing.

Given that I have a solution that is working well enough for me, I don't have the desire (or time) to investigate a different approach that I've already failed at. If the pull request isn't accepted, I'll just continue to use it in my own fork, and everyone else will have to continue with the current behaviour (frozen UI) unless someone else steps up with an alternative.

[sledgehammer999]

@magao do you think this PR will benefit about the tips I gave you in #6177 about processEvent() and a single timer?

Btw, what action is that slows down the UI here?

[magao]

Yes - this will benefit from those tips.

There are 2 cases to deal with:

1. At startup Rss::Feed::loadItemsFromDisk() operates on the UI thread and calls addArticle() many times. Each article goes through filter matching, and then adding trackers, etc if matched. During this period the UI is frozen.

2. When an RSS feed is updated, it is parsed on a separate thread. However, as I understand how the signals and slots are set up, when the parser emits a newArticle signal, processing moves to the UI thread where the article goes through filter matching, adding trackers, etc. If a bunch of signals are emitted in quick succession then they all have to be processed before any UI events.

So what I've done is made it so that we only process a single article before allowing UI and other events to progress, then process the next article, etc.

[sledgehammer999]

When an RSS feed is updated, it is parsed on a separate thread. However, as I understand how the signals and slots are set up, when the parser emits a newArticle signal, processing moves to the UI thread where the article goes through filter matching, adding trackers, etc. If a bunch of signals are emitted in quick succession then they all have to be processed before any UI events.

Hmm, then that means that in the future we should cache those signals and delay them. Then have the appropriate UI element ask the backend(after application startup): "hey I have finished starting up. Do you have feeds parsed/updated? Give them to me in one go".

[magao]

That approach would still result in a UI freeze while the articles are being processed, it would just happen later.

The only methods I can see that won't result in a UI freeze is to either:

1. DO all the RSS processing on the RSS parser thread (this would be my preference, but I couldn't get it to work);

2. Process the articles in batches (1 at a time like this PR, or batches taking no more than 250ms, etc).

[sledgehammer999]

Just to clarify, I am not expecting YOU do it now. Just polish up this PR according to my other comments. This PR is a quick fix.

[magao]

@sledgehammer999 Rebased on top of master. Reworked to use single QTimer for all feeds, dispatched via Rss::Manager (otherwise having lots of feeds update at once could cause minor UI freezes).

[magao]

Rebased on top of master.

[sledgehammer999]

Thx. I hope that this works as intended. I am not an RSS user so I can't fully test it.