[Scheduler] Use flag to track when a runner is going idle
ClosedPublic

Authored by bruns on Oct 16 2018, 10:12 PM.

Details

Summary

Only one runner should be queued/running at a given time, but the current
condition "m_threadPool.activeThreadCount == 0" is to strong, as the
done() signal is fired from the runner thread while it is still alive and
the signal may be processed before the thread has exited.

Set a flag when the runner is finished, and use this to guard from
scheduling/enqueuing multiple runners. It is fine to enqueue another
runner while the old is cleaning up, the new one will become active
afterwards.

Alternative approach to D15959.

Test Plan

start `balooctl monitor~
touch a file -> file is indexed, scheduler goes idle afterwards
touch multiple files, new files -> indexing, idle

Diff Detail

Repository
R293 Baloo
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bruns created this revision.Oct 16 2018, 10:12 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptOct 16 2018, 10:12 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
bruns requested review of this revision.Oct 16 2018, 10:12 PM

I like it, it's better than D15959: Wait for the extraction process to finish before scheduling.
And it seems to be working, as far as I can see :)

src/file/fileindexscheduler.h
57–59

But the very same check is already in the first line of scheduleIndexing, why do we need it also here?

bruns added a comment.Oct 20 2018, 6:28 PM

So now we just need someone who accepts it 😏

bruns marked an inline comment as done.Oct 20 2018, 10:47 PM
bruns added inline comments.
src/file/fileindexscheduler.h
57–59

Not strictly necessary, but avoids to schedule a timer we will discard anyway

bruns edited the summary of this revision. (Show Details)Oct 23 2018, 1:29 PM
lbeltrame accepted this revision.Oct 24 2018, 7:12 AM
lbeltrame added a subscriber: lbeltrame.

The changes look sane to me. Perhaps wait a couple more days until any other objection is raised, then if not, commit away.

This revision is now accepted and ready to land.Oct 24 2018, 7:12 AM
This revision was automatically updated to reflect the committed changes.
bruns marked an inline comment as done.