narvaez abandoned this revision.
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D19007
To: narvaez, #baloo, bruns, astippich, poboiko
Cc: davidedmundson, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh,
astippich, spoorun, ngraham, bruns, abrahams
bruns added inline comments.
INLINE COMMENTS
> narvaez wrote in monitor.cpp:90
> The batch size is **hardcoded** at 40 so querying it at at any interval is
> pointless, for that matter. Instead of hardcoding more magic numbers I would
> just #define BATCH_SIZE 40 inside the Engine lib and set t
narvaez added inline comments.
INLINE COMMENTS
> bruns wrote in monitor.cpp:90
> The batchsize 40 by default, so currently the update interval is 200 files.
> 100 or 200 does not matter to much, it is just a crude limiter anyway.
>
> A better fix would be emit the remaining time directly from t
bruns added inline comments.
INLINE COMMENTS
> narvaez wrote in monitor.cpp:90
> It does fix the issue because if getBatchSize returns 0 then updateInterval
> is set to 100. It does introduce another issue which is that it won't allow
> for an update interval under 100 (say, if the batch size w
narvaez added inline comments.
INLINE COMMENTS
> davidedmundson wrote in monitor.cpp:90
> That won't fix the issue.
>
> getBatchSize is still returning a QDBusPendingReply, using the value before
> it's loaded will always be zero.
It does fix the issue because if getBatchSize returns 0 then up
davidedmundson added inline comments.
INLINE COMMENTS
> bruns wrote in monitor.cpp:90
> much more trivial:
>
> auto updateInterval = std::max(100, 5 * m_scheduler->getBatchSize());
> if (m_filesIndexed % updateInterval == 0) { ...
That won't fix the issue.
getBatchSize is still returning a
bruns requested changes to this revision.
This revision now requires changes to proceed.
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D19007
To: narvaez, #baloo, bruns, astippich, poboiko
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngr
bruns added a comment.
good catch, but the fix is overengineered ...
INLINE COMMENTS
> monitor.cpp:90
> +batchSize.waitForFinished();
>
> +if (!batchSize.isError()) {
much more trivial:
auto updateInterval = std::max(100, 5 * m_scheduler->getBatchSize());
if (m_filesIndexed %
ngraham added reviewers: bruns, astippich, poboiko.
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D19007
To: narvaez, #baloo, bruns, astippich, poboiko
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
narvaez edited the test plan for this revision.
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D19007
To: narvaez, #baloo
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
narvaez added a reviewer: Baloo.
REPOSITORY
R293 Baloo
REVISION DETAIL
https://phabricator.kde.org/D19007
To: narvaez, #baloo
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,
ngraham, bruns, abrahams
narvaez created this revision.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
narvaez requested review of this revision.
REVISION SUMMARY
If the scheduler has not replied by the time the batch size is used, the
default value is 0 and the mod op
12 matches
Mail list logo
