bruns added inline comments.

INLINE COMMENTS

> databasesanitizer.cpp:159
> +        // Create a hash to sum-up indexed items
>          QMultiHash<quint32, FileInfo> usedDevices;
> +        for (const auto& info : infos) {

I can not come up with a reason to use a MultiHash here in the first place - 
all that is needed is the deviceId and the file count per device.

You can use a QMap<DeviceId, Count> here, and use

  usedDevices[info.deviceId] += 1;

(operator[] default-constructs the ValueType, which for e.g. `int` is guranteed 
by Qt to be 0).

This reduces the effort for counting the items per device from //(D + 3) * F// 
to //F//.
(D: number of devices, F: number of files).
Currently you walk the infos list once to create the MultiHash, and then walk 
the Multihash once for `uniqueKeys()`, and once for each device when calling 
`usedDevices.value()`. Cumulated effort for `.count()` is //F//.

In your case, this would reduce the time for counting by a factor of 14. You 
also save the memory for creating a temporary MultiHash with ~20000 items.

> michaelh wrote in fsutils.cpp:111
> Sure? We would have to add Solid as a dependency for `libKF5BalooEngine.so`. 
> `databasesanitizer` currently is experimental not exported by default (You 
> cannot see this because I haven't rebase yet.).
> I did not use Solid, because I don't know it well and it seemed easier to 
> copy and adapt `FSUtils::getDirectoryFileSystem`. I'm not objecting to use 
> Solid though.
> 
> The true reason for 'fsutils`'s existence is to disable CoW on btrfs. Can 
> Solid do that? If so we can drop `fsutils`.

The question is if this really belongs in the library then.

IMHO the whole `printDevices()` function should be moved into baloodb.cpp, The 
DatabaseSanitizer should export either `createList` or a function returning 
`QVector<DeviceId, Count>`. Going from the DeviceId to the DeviceInfo should 
happen in baloodb.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D11452

To: michaelh, #baloo, #frameworks
Cc: bruns, smithjd, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
alexeymin

Reply via email to