[KDevelop] background parser job control
Needs ReviewPublic

Authored by rjvbb on Mar 14 2020, 10:30 AM.

Details

Reviewers
aaronpuchert
Group Reviewers
KDevelop
Summary

As discussed elsewhere, KDevelop doesn't currently provide a means of job control over the background parser, except when parsing a complete project.

This WIP patch introduces the simplest form of such control: a possibility to stop all background parsing processes that are not part of a project (re)parse. This includes documents that were part of such a (re)parse, got parsed, and then were rescheduled to be parsed again.

The underlying approach I used consists of an instance of a proxy/stub job controller (one per background parser instance, of which there is apparently never more than 1 at the moment). This proxy and the BGParser d class keep track of which documents are under control of the proxy so that only those jobs are cancelled at the user's request.

Most of the change is transparent outside of the BackgroundParser implementation (and doesn't cost much as far as I can tell) but I did have to add a new method to that class for client code that already implements its own job control.

Initially I just dequeued all concerned ThreadWeaver jobs but this caused intermittent crashes that looked as if they were caused by a race condition if one of those jobs was actually running. This was addressed by suspending the weaver first, which also means that the running job is allowed to finish first - not ideal but I haven't found a safe way around that yet.

As a side-effect this change also avoids queuing a reparse of a document that is already queued for parsing, which is probably a good thing.

An alternative would be to implement a "cancel all background parsing" action, but this would also need to cancel the project parse jobs and I don't really see how to do that properly without using the current approach and then sending abort commands to all JobController entries.

Test Plan

The easiest way to test this is to load a session that restores multiple documents op launch, and open the "menu allowing to stop individual jobs". Or click the "Revert all open documents" toolbar button (which really should be called "reload"! ;)) and then open the run controller menu. Keep it open, the document count is updated. Click the entry to stop background processing (= cancel all pending parse jobs - cancelling the current job cannot be done safely it seems).

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 25076
Build 25094: arc lint + arc unit
rjvbb created this revision.Mar 14 2020, 10:30 AM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptMar 14 2020, 10:30 AM
rjvbb requested review of this revision.Mar 14 2020, 10:30 AM
rjvbb updated this revision to Diff 77846.Mar 17 2020, 4:53 PM

This addresses a few issues, notably by using a queued signal to trigger url (un)registering from the requesting (weaver) thread in the thread where the JobController lives too.

The errors resulting from not doing that became apparently only when I tested this with the full-project-parse-on-load option turned on.

rjvbb set the repository for this revision to R32 KDevelop.Mar 17 2020, 4:53 PM
rjvbb updated this revision to Diff 79868.Apr 11 2020, 3:20 PM

Updated and improved version:

  • the registration with the ProcessController is done "JIT", just before enqueing the first document for actual parsing
  • keep track of documents via the QUrls of IndexedStrings

Both changes are necessary because the first set of documents to be parsed (= the documents restored on session launch?) may not actually get parsed (I see that with a session holding TCL-based projects), and that first set will not be deleted until the session exits. Then, BackgroundParser::documentClosed calls the proxy's unregisterUrl() method via a queued connection and those calls will be performed much later, after the Core cleanup handler has exitted. By then the IndexedString references are no longer safe to use, hence the need to store deep (QUrl) copies of them.
Registering in JIT fashion with the ProcessController prevents a persistent job entry that I was seeing in sessions where none of the initial documents were actually parsed, and which caused instability if an attempt was made to cancel it via the ProcessController.

rjvbb set the repository for this revision to R32 KDevelop.Apr 11 2020, 3:21 PM
rjvbb updated this revision to Diff 79933.Apr 12 2020, 2:55 PM

Don't just stop parsing files when shutting down, also ignore new document add requests.

rjvbb set the repository for this revision to R32 KDevelop.Apr 12 2020, 2:55 PM
rjvbb updated this revision to Diff 81418.Apr 28 2020, 9:02 AM

I've been addressing issues related to controller proxy instances surviving too long during shutdown and accessing stale memory.

The current version disables the proxy, removes it from the runController and schedules itself for ASAP deletion when the shutdown notification arrives.

rjvbb retitled this revision from [KDevelop] background parser job control (WIP) to [KDevelop] background parser job control.Apr 28 2020, 9:08 AM
rjvbb edited the test plan for this revision. (Show Details)
rjvbb set the repository for this revision to R32 KDevelop.