Export the StorageDevices object.
Needs RevisionPublic

Authored by smithjd on Aug 24 2018, 1:46 AM.

Details

Reviewers
broulik
bruns
poboiko
Group Reviewers
Baloo

Diff Detail

Repository
R293 Baloo
Branch
master-storageDevicesExport (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2182
Build 2200: arc lint + arc unit
smithjd created this revision.Aug 24 2018, 1:46 AM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptAug 24 2018, 1:46 AM
Restricted Application added subscribers: Baloo, kde-frameworks-devel. · View Herald Transcript
smithjd requested review of this revision.Aug 24 2018, 1:46 AM

This was included in https://phabricator.kde.org/D11529 and is now broken out.

bruns added a subscriber: bruns.Aug 24 2018, 2:07 AM

There is no need for this, just create a new StorageDevices where you need it.
Creating a second StorageDevices instance in a process is quite cheap.

src/file/fileindexerconfig.cpp
322

This is bad, you remove the const from the QList<>
Read up about Qt and detaching ...

smithjd marked an inline comment as done.Aug 24 2018, 2:49 AM

There is no need for this, just create a new StorageDevices where you need it.
Creating a second StorageDevices instance in a process is quite cheap.

Creating a separate object cluttered the console with duplicated debug output and raised a threading error.

src/file/fileindexerconfig.cpp
322

According to https://www.dvratil.cz/2015/06/qt-containers-and-c11-range-based-loops this particular example will detach with or without the const intermediate.

bruns added inline comments.Aug 24 2018, 1:42 PM
src/file/fileindexerconfig.cpp
322

Citing from above:

To sum this up, when using range-based loops with Qt containers:
Make sure the container is const …
… or make sure the container is not shared.

bruns added a comment.Aug 24 2018, 1:46 PM

There is no need for this, just create a new StorageDevices where you need it.
Creating a second StorageDevices instance in a process is quite cheap.

Creating a separate object cluttered the console with duplicated debug output and raised a threading error.

Then your code is wrong anyway. This is lazy initialization, and if you can not not guarantee it is save to do so, don't do it.

There is no need for this, just create a new StorageDevices where you need it.
Creating a second StorageDevices instance in a process is quite cheap.

Creating a separate object cluttered the console with duplicated debug output and raised a threading error.

Then your code is wrong anyway. This is lazy initialization, and if you can not not guarantee it is save to do so, don't do it.

Duplicated debug output is also undesireable...

smithjd updated this revision to Diff 43353.Oct 10 2018, 9:00 PM
  • Re-write the file index scheduler. Combine content indexer suspend logic.
  • Update the balooctl tool with the changed suspend/resume behaviour.
  • Re-order and use IndexerState to prioritize the indexer thread pool.
  • Newline and tab fixes.
  • Prevent a scheduler race condition.
  • Separate runnableStateChanged into two functors.
  • Allow the first run indexer to complete before running any other runnables. Prevent the new file and unindexed file runnables from running at the same time.
  • Revert "Re-order and use IndexerState to prioritize the indexer thread pool."
  • Simplify locking the scheduler.
  • Adapt index cleaner to scheduler.
smithjd updated this revision to Diff 43357.Oct 10 2018, 9:07 PM
  • Rebase.
bruns requested changes to this revision.Oct 10 2018, 9:52 PM

You still have not addressed the raised issues. This gets annoying ...

This revision now requires changes to proceed.Oct 10 2018, 9:52 PM