Suspend the background parser during Core initialization
ClosedPublic

Authored by mwolff on Nov 19 2017, 5:32 PM.

Details

Summary

When KDevelop starts up, we will quite early receive the background
parse requests for the last opened documents in the session. These
then often get added to the background parser (BP) before the project
controller is fully initialized - i.e. before any project was even
tried to be loaded. This means the m_loadingProjects safety guard in
the BP does not kick in and we create parse jobs with "broken"
environments, i.e. missing include paths and defines for kdev-clang.
If, and only if, the document has been modified since the last time
KDevelop was started, we would actually parse the document with the
broken environment, leading to flickering when we show the result
for the broken environment before reparsing it after the project gets
opened with the correct environment. Additionally, this of course
adds useless load to the startup phase and frees up resources better
spent elsewhere at this point.

Note that since the project controller itself delays its
initialization via the eventloop, we have to add a new signal to
ensure we resume the background parser only once we have tried to
open all previous projects, such that the m_loadingProjects safety
guard in the BP can kick in properly.

This is not fully integration-tested, as it's pretty complicated
to do so. Rather, we only test the individual parts automatically.
The full integration test was done manually via:

  • create kdev session and open some project
  • open a file in that project
  • close session with file still opened
  • touch file on disk to force reload on next startup
  • restart kdevelop session
  • check visually and via CLI output whether file gets reparsed twice, i.e. before the project was loaded and afterwards

This patch series ensures it only gets reparsed once after the project
finished importing.

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mwolff created this revision.Nov 19 2017, 5:32 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptNov 19 2017, 5:32 PM
brauch accepted this revision.Nov 19 2017, 6:06 PM
brauch added a subscriber: brauch.

Makes sense to me. Thanks for working on this issue!

kdevplatform/shell/core.cpp
248 ↗(On Diff #22609)

no need for the .data(), is there?

kdevplatform/shell/projectcontroller.cpp
663 ↗(On Diff #22609)

if you use Q_SIGNALS it would be consequent to use Q_EMIT ;)

This revision is now accepted and ready to land.Nov 19 2017, 6:06 PM
mwolff added inline comments.Nov 19 2017, 6:46 PM
kdevplatform/shell/core.cpp
248 ↗(On Diff #22609)

true, I'll fix this up everywhere in a separate patch

kdevplatform/shell/projectcontroller.cpp
663 ↗(On Diff #22609)

afaik we only require that for our headers, but not for the .cpp? the header also uses Q_SLOTS btw

Ah yes, good point about Q_SIGNALS, makes sense.

This revision was automatically updated to reflect the committed changes.