ILocatorFilter: Replace refresh() with refreshRecipe()

Most of refresh overrides were running synchronous method in main
thread, however, it was really convoluted: we were starting
asynchronous call and from inside the method running in separate
thread we were scheduling the call back to main thread.
Use Tasking::Sync for these cases instead.

The only subclass that is doing the real asynchronous refresh
is DirectoryFilter. Simplify the async call internals and
pass data to the async call explicitly. Move the refresh method
outside of DirectoryFilter class (to make it clear we shouldn't
touch private members).

Change-Id: I6af788611bdc49db1ff812f91202ac40a280fbc8
Reviewed-by: Eike Ziller <eike.ziller@qt.io>
This commit is contained in:
Jarek Kobus
2023-02-06 22:59:30 +01:00
parent 913513ff62
commit ce457a9cdc
16 changed files with 151 additions and 149 deletions

View File

@@ -7,12 +7,12 @@
#include "../coreplugintr.h"
#include <utils/algorithm.h>
#include <utils/asynctask.h>
#include <utils/fileutils.h>
#include <utils/filesearch.h>
#include <utils/layoutbuilder.h>
#include <QCheckBox>
#include <QCoreApplication>
#include <QDialog>
#include <QDialogButtonBox>
#include <QJsonArray>
@@ -60,8 +60,6 @@ DirectoryFilter::DirectoryFilter(Id id)
void DirectoryFilter::saveState(QJsonObject &object) const
{
QMutexLocker locker(&m_lock); // m_files is modified in other thread
if (displayName() != defaultDisplayName())
object.insert(kDisplayNameKey, displayName());
if (!m_directories.isEmpty()) {
@@ -92,7 +90,6 @@ static FilePaths toFilePaths(const QJsonArray &array)
void DirectoryFilter::restoreState(const QJsonObject &object)
{
QMutexLocker locker(&m_lock);
setDisplayName(object.value(kDisplayNameKey).toString(defaultDisplayName()));
m_directories = toFilePaths(object.value(kDirectoriesKey).toArray());
m_filters = toStringList(
@@ -107,8 +104,6 @@ void DirectoryFilter::restoreState(const QByteArray &state)
{
if (isOldSetting(state)) {
// TODO read old settings, remove some time after Qt Creator 4.15
QMutexLocker locker(&m_lock);
QString name;
QStringList directories;
QString shortcut;
@@ -136,8 +131,6 @@ void DirectoryFilter::restoreState(const QByteArray &state)
setDisplayName(name);
setShortcutString(shortcut);
setIncludedByDefault(defaultFilter);
locker.unlock();
} else {
ILocatorFilter::restoreState(state);
}
@@ -263,8 +256,6 @@ bool DirectoryFilter::openConfigDialog(QWidget *parent, bool &needsRefresh)
&DirectoryFilter::updateOptionButtons,
Qt::DirectConnection);
m_dialog->directoryList->clear();
// Note: assuming we only change m_directories in the Gui thread,
// we don't need to protect it here with mutex
m_dialog->directoryList->addItems(Utils::transform(m_directories, &FilePath::toString));
m_dialog->nameLabel->setVisible(m_isCustomFilter);
m_dialog->nameEdit->setVisible(m_isCustomFilter);
@@ -276,14 +267,10 @@ bool DirectoryFilter::openConfigDialog(QWidget *parent, bool &needsRefresh)
m_dialog->filePatternLabel->setText(Utils::msgFilePatternLabel());
m_dialog->filePatternLabel->setBuddy(m_dialog->filePattern);
m_dialog->filePattern->setToolTip(Utils::msgFilePatternToolTip());
// Note: assuming we only change m_filters in the Gui thread,
// we don't need to protect it here with mutex
m_dialog->filePattern->setText(Utils::transform(m_filters, &QDir::toNativeSeparators).join(','));
m_dialog->exclusionPatternLabel->setText(Utils::msgExclusionPatternLabel());
m_dialog->exclusionPatternLabel->setBuddy(m_dialog->exclusionPattern);
m_dialog->exclusionPattern->setToolTip(Utils::msgFilePatternToolTip());
// Note: assuming we only change m_exclusionFilters in the Gui thread,
// we don't need to protect it here with mutex
m_dialog->exclusionPattern->setText(
Utils::transform(m_exclusionFilters, &QDir::toNativeSeparators).join(','));
m_dialog->shortcutEdit->setText(shortcutString());
@@ -291,7 +278,6 @@ bool DirectoryFilter::openConfigDialog(QWidget *parent, bool &needsRefresh)
updateOptionButtons();
dialog.adjustSize();
if (dialog.exec() == QDialog::Accepted) {
QMutexLocker locker(&m_lock);
bool directoriesChanged = false;
const FilePaths oldDirectories = m_directories;
const QStringList oldFilters = m_filters;
@@ -353,53 +339,9 @@ void DirectoryFilter::updateOptionButtons()
void DirectoryFilter::updateFileIterator()
{
QMutexLocker locker(&m_lock);
setFileIterator(new BaseFileFilter::ListIterator(m_files));
}
void DirectoryFilter::refresh(QFutureInterface<void> &future)
{
FilePaths directories;
QStringList filters, exclusionFilters;
{
QMutexLocker locker(&m_lock);
if (m_directories.isEmpty()) {
m_files.clear();
QMetaObject::invokeMethod(this, &DirectoryFilter::updateFileIterator,
Qt::QueuedConnection);
future.setProgressRange(0, 1);
future.setProgressValueAndText(1, Tr::tr("%1 filter update: 0 files").arg(displayName()));
return;
}
directories = m_directories;
filters = m_filters;
exclusionFilters = m_exclusionFilters;
}
Utils::SubDirFileIterator subDirIterator(directories, filters, exclusionFilters);
future.setProgressRange(0, subDirIterator.maxProgress());
Utils::FilePaths filesFound;
auto end = subDirIterator.end();
for (auto it = subDirIterator.begin(); it != end; ++it) {
if (future.isCanceled())
break;
filesFound << (*it).filePath;
if (future.isProgressUpdateNeeded()
|| future.progressValue() == 0 /*workaround for regression in Qt*/) {
future.setProgressValueAndText(subDirIterator.currentProgress(),
Tr::tr("%1 filter update: %n files", nullptr, filesFound.size()).arg(displayName()));
}
}
if (!future.isCanceled()) {
QMutexLocker locker(&m_lock);
m_files = filesFound;
QMetaObject::invokeMethod(this, &DirectoryFilter::updateFileIterator, Qt::QueuedConnection);
future.setProgressValue(subDirIterator.maxProgress());
} else {
future.setProgressValueAndText(subDirIterator.currentProgress(), Tr::tr("%1 filter update: canceled").arg(displayName()));
}
}
void DirectoryFilter::setIsCustomFilter(bool value)
{
m_isCustomFilter = value;
@@ -409,10 +351,7 @@ void DirectoryFilter::setDirectories(const FilePaths &directories)
{
if (directories == m_directories)
return;
{
QMutexLocker locker(&m_lock);
m_directories = directories;
}
m_directories = directories;
Internal::Locator::instance()->refresh({this});
}
@@ -435,14 +374,54 @@ FilePaths DirectoryFilter::directories() const
void DirectoryFilter::setFilters(const QStringList &filters)
{
QMutexLocker locker(&m_lock);
m_filters = filters;
}
void DirectoryFilter::setExclusionFilters(const QStringList &exclusionFilters)
{
QMutexLocker locker(&m_lock);
m_exclusionFilters = exclusionFilters;
}
static void refresh(QFutureInterface<FilePaths> &future, const FilePaths &directories,
const QStringList &filters, const QStringList &exclusionFilters,
const QString &displayName)
{
if (directories.isEmpty())
return;
SubDirFileIterator subDirIterator(directories, filters, exclusionFilters);
future.setProgressRange(0, subDirIterator.maxProgress());
FilePaths files;
const auto end = subDirIterator.end();
for (auto it = subDirIterator.begin(); it != end; ++it) {
if (future.isCanceled()) {
future.setProgressValueAndText(subDirIterator.currentProgress(),
Tr::tr("%1 filter update: canceled").arg(displayName));
return;
}
files << (*it).filePath;
if (future.isProgressUpdateNeeded() || future.progressValue() == 0) {
future.setProgressValueAndText(subDirIterator.currentProgress(),
Tr::tr("%1 filter update: %n files", nullptr, files.size()).arg(displayName));
}
}
future.setProgressValue(subDirIterator.maxProgress());
future.reportResult(files);
}
using namespace Utils::Tasking;
std::optional<TaskItem> DirectoryFilter::refreshRecipe()
{
const auto setup = [this](AsyncTask<FilePaths> &async) {
async.setAsyncCallData(&refresh, m_directories, m_filters, m_exclusionFilters,
displayName());
};
const auto done = [this](const AsyncTask<FilePaths> &async) {
m_files = async.isResultAvailable() ? async.result() : FilePaths();
updateFileIterator();
};
return Async<FilePaths>(setup, done);
}
} // namespace Core

View File

@@ -7,10 +7,7 @@
#include <coreplugin/core_global.h>
#include <QString>
#include <QByteArray>
#include <QFutureInterface>
#include <QMutex>
namespace Core {
@@ -22,7 +19,6 @@ public:
DirectoryFilter(Utils::Id id);
void restoreState(const QByteArray &state) override;
bool openConfigDialog(QWidget *parent, bool &needsRefresh) override;
void refresh(QFutureInterface<void> &future) override;
void setIsCustomFilter(bool value);
void setDirectories(const Utils::FilePaths &directories);
@@ -42,6 +38,7 @@ private:
void handleRemoveDirectory();
void updateOptionButtons();
void updateFileIterator();
std::optional<Utils::Tasking::TaskItem> refreshRecipe() override;
Utils::FilePaths m_directories;
QStringList m_filters;
@@ -49,7 +46,6 @@ private:
// Our config dialog, uses in addDirectory and editDirectory
// to give their dialogs the right parent
class DirectoryFilterOptions *m_dialog = nullptr;
mutable QMutex m_lock;
Utils::FilePaths m_files;
bool m_isCustomFilter = true;
};

View File

@@ -7,6 +7,7 @@
#include "../editormanager/editormanager.h"
#include <utils/fuzzymatcher.h>
#include <utils/tasktree.h>
#include <QBoxLayout>
#include <QCheckBox>
@@ -99,6 +100,15 @@ void ILocatorFilter::prepareSearch(const QString &entry)
Q_UNUSED(entry)
}
/*!
Returns refresh recipe for refreshing cached data. By default, no recipe is returned, so
that the filter won't be refreshed.
*/
std::optional<Tasking::TaskItem> ILocatorFilter::refreshRecipe()
{
return {};
}
/*!
Called with the entry specified by \a selection when the user activates it
in the result list.
@@ -220,13 +230,13 @@ void ILocatorFilter::restoreState(const QByteArray &state)
various aspects of the filter. Called when the user requests to configure
the filter.
Set \a needsRefresh to \c true, if a refresh() should be done after
Set \a needsRefresh to \c true, if a refresh should be done after
closing the dialog. Return \c false if the user canceled the dialog.
The default implementation allows changing the shortcut and whether the
filter is included by default.
\sa refresh()
\sa refreshRecipe()
*/
bool ILocatorFilter::openConfigDialog(QWidget *parent, bool &needsRefresh)
{
@@ -616,14 +626,6 @@ bool ILocatorFilter::isOldSetting(const QByteArray &state)
\sa caseSensitivity()
*/
/*!
\fn void Core::ILocatorFilter::refresh(QFutureInterface<void> &future)
Refreshes cached data asynchronously.
If \a future is \c canceled, the refresh should be aborted.
*/
/*!
\enum Core::ILocatorFilter::Priority

View File

@@ -17,8 +17,12 @@
#include <optional>
namespace Utils::Tasking { class TaskItem; }
namespace Core {
namespace Internal { class Locator; }
class ILocatorFilter;
class LocatorFilterEntry
@@ -157,8 +161,6 @@ public:
virtual void accept(const LocatorFilterEntry &selection, QString *newText,
int *selectionStart, int *selectionLength) const;
virtual void refresh(QFutureInterface<void> &future) { Q_UNUSED(future) };
virtual QByteArray saveState() const;
virtual void restoreState(const QByteArray &state);
@@ -202,6 +204,9 @@ protected:
static bool isOldSetting(const QByteArray &state);
private:
friend class Internal::Locator;
virtual std::optional<Utils::Tasking::TaskItem> refreshRecipe();
Utils::Id m_id;
QString m_shortcut;
Priority m_priority = Medium;

View File

@@ -382,14 +382,16 @@ void Locator::refresh(const QList<ILocatorFilter *> &filters)
using namespace Tasking;
QList<TaskItem> tasks{parallel};
for (ILocatorFilter *filter : std::as_const(m_refreshingFilters)) {
const auto setupRefresh = [filter](AsyncTask<void> &async) {
async.setAsyncCallData(&ILocatorFilter::refresh, filter);
const auto task = filter->refreshRecipe();
if (!task.has_value())
continue;
const Group group {
optional,
*task,
OnGroupDone([this, filter] { m_refreshingFilters.removeOne(filter); })
};
const auto onRefreshDone = [this, filter](const AsyncTask<void> &async) {
Q_UNUSED(async)
m_refreshingFilters.removeOne(filter);
};
tasks.append(Async<void>(setupRefresh, onRefreshDone));
tasks.append(group);
}
m_taskTree.reset(new TaskTree{tasks});

View File

@@ -28,8 +28,6 @@ public:
{
setFileIterator(new BaseFileFilter::ListIterator(theFiles));
}
void refresh(QFutureInterface<void> &) override {}
};
class ReferenceData

View File

@@ -7,6 +7,7 @@
#include <utils/filepath.h>
#include <utils/link.h>
#include <utils/tasktree.h>
#include <QAbstractItemModel>
#include <QMutexLocker>
@@ -115,4 +116,22 @@ QList<OpenDocumentsFilter::Entry> OpenDocumentsFilter::editors() const
return m_editors;
}
void OpenDocumentsFilter::refreshInternally()
{
QMutexLocker lock(&m_mutex);
m_editors.clear();
const QList<DocumentModel::Entry *> documentEntries = DocumentModel::entries();
// create copy with only the information relevant to use
// to avoid model deleting entries behind our back
for (DocumentModel::Entry *e : documentEntries)
m_editors.append({e->filePath(), e->displayName()});
}
using namespace Utils::Tasking;
std::optional<TaskItem> OpenDocumentsFilter::refreshRecipe()
{
return Sync([this] { refreshInternally(); return true; });
}
} // Core::Internal

View File

@@ -7,10 +7,7 @@
#include <coreplugin/editormanager/documentmodel.h>
#include <QFutureInterface>
#include <QList>
#include <QMutex>
#include <QString>
namespace Core {
namespace Internal {
@@ -38,6 +35,8 @@ private:
};
QList<Entry> editors() const;
void refreshInternally();
std::optional<Utils::Tasking::TaskItem> refreshRecipe() override;
mutable QMutex m_mutex;
QList<Entry> m_editors;

View File

@@ -14,6 +14,8 @@
#include <projectexplorer/project.h>
#include <projectexplorer/projectexplorer.h>
#include <projectexplorer/projectmanager.h>
#include <projectexplorer/session.h>
#include <utils/tasktree.h>
using namespace Core;
using namespace ProjectExplorer;
@@ -109,19 +111,19 @@ CppIncludesFilter::CppIncludesFilter()
setPriority(ILocatorFilter::Low);
connect(ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::fileListChanged,
this, &CppIncludesFilter::markOutdated);
this, &CppIncludesFilter::invalidateCache);
connect(CppModelManager::instance(), &CppModelManager::documentUpdated,
this, &CppIncludesFilter::markOutdated);
this, &CppIncludesFilter::invalidateCache);
connect(CppModelManager::instance(), &CppModelManager::aboutToRemoveFiles,
this, &CppIncludesFilter::markOutdated);
this, &CppIncludesFilter::invalidateCache);
connect(DocumentModel::model(), &QAbstractItemModel::rowsInserted,
this, &CppIncludesFilter::markOutdated);
this, &CppIncludesFilter::invalidateCache);
connect(DocumentModel::model(), &QAbstractItemModel::rowsRemoved,
this, &CppIncludesFilter::markOutdated);
this, &CppIncludesFilter::invalidateCache);
connect(DocumentModel::model(), &QAbstractItemModel::dataChanged,
this, &CppIncludesFilter::markOutdated);
this, &CppIncludesFilter::invalidateCache);
connect(DocumentModel::model(), &QAbstractItemModel::modelReset,
this, &CppIncludesFilter::markOutdated);
this, &CppIncludesFilter::invalidateCache);
}
void CppIncludesFilter::prepareSearch(const QString &entry)
@@ -146,16 +148,17 @@ void CppIncludesFilter::prepareSearch(const QString &entry)
BaseFileFilter::prepareSearch(entry);
}
void CppIncludesFilter::refresh(QFutureInterface<void> &future)
{
Q_UNUSED(future)
QMetaObject::invokeMethod(this, &CppIncludesFilter::markOutdated, Qt::QueuedConnection);
}
void CppIncludesFilter::markOutdated()
void CppIncludesFilter::invalidateCache()
{
m_needsUpdate = true;
setFileIterator(nullptr); // clean up
}
using namespace Utils::Tasking;
std::optional<TaskItem> CppIncludesFilter::refreshRecipe()
{
return Sync([this] { invalidateCache(); return true; });
}
} // namespace CppEditor::Internal

View File

@@ -15,10 +15,10 @@ public:
// ILocatorFilter interface
public:
void prepareSearch(const QString &entry) override;
void refresh(QFutureInterface<void> &future) override;
private:
void markOutdated();
void invalidateCache();
std::optional<Utils::Tasking::TaskItem> refreshRecipe() override;
bool m_needsUpdate = true;
};

View File

@@ -11,6 +11,7 @@
#include <coreplugin/helpmanager.h>
#include <extensionsystem/pluginmanager.h>
#include <utils/utilsicons.h>
#include <utils/tasktree.h>
#include <QHelpEngine>
#include <QHelpFilterEngine>
@@ -105,12 +106,6 @@ void HelpIndexFilter::accept(const LocatorFilterEntry &selection,
emit linksActivated(links, key);
}
void HelpIndexFilter::refresh(QFutureInterface<void> &future)
{
Q_UNUSED(future)
invalidateCache();
}
QStringList HelpIndexFilter::allIndices() const
{
LocalHelpManager::setupGuiHelpEngine();
@@ -121,3 +116,10 @@ void HelpIndexFilter::invalidateCache()
{
m_needsUpdate = true;
}
using namespace Utils::Tasking;
std::optional<TaskItem> HelpIndexFilter::refreshRecipe()
{
return Sync([this] { invalidateCache(); return true; });
}

View File

@@ -7,7 +7,6 @@
#include <QIcon>
#include <QMultiMap>
#include <QSet>
#include <QUrl>
#include <atomic>
@@ -28,7 +27,6 @@ public:
const QString &entry) override;
void accept(const Core::LocatorFilterEntry &selection,
QString *newText, int *selectionStart, int *selectionLength) const override;
void refresh(QFutureInterface<void> &future) override;
QStringList allIndices() const;
@@ -36,10 +34,10 @@ signals:
void linksActivated(const QMultiMap<QString, QUrl> &links, const QString &key) const;
private:
void invalidateCache();
bool updateCache(QFutureInterface<Core::LocatorFilterEntry> &future,
const QStringList &cache, const QString &entry);
void invalidateCache();
std::optional<Utils::Tasking::TaskItem> refreshRecipe() override;
QStringList m_allIndicesCache;
QStringList m_lastIndicesCache;

View File

@@ -9,6 +9,7 @@
#include "projectmanager.h"
#include <utils/algorithm.h>
#include <utils/tasktree.h>
using namespace Core;
@@ -25,12 +26,7 @@ AllProjectsFilter::AllProjectsFilter()
setDefaultIncludedByDefault(true);
connect(ProjectExplorerPlugin::instance(), &ProjectExplorerPlugin::fileListChanged,
this, &AllProjectsFilter::markFilesAsOutOfDate);
}
void AllProjectsFilter::markFilesAsOutOfDate()
{
setFileIterator(nullptr);
this, &AllProjectsFilter::invalidateCache);
}
void AllProjectsFilter::prepareSearch(const QString &entry)
@@ -46,10 +42,16 @@ void AllProjectsFilter::prepareSearch(const QString &entry)
BaseFileFilter::prepareSearch(entry);
}
void AllProjectsFilter::refresh(QFutureInterface<void> &future)
void AllProjectsFilter::invalidateCache()
{
Q_UNUSED(future)
QMetaObject::invokeMethod(this, &AllProjectsFilter::markFilesAsOutOfDate, Qt::QueuedConnection);
setFileIterator(nullptr);
}
using namespace Utils::Tasking;
std::optional<TaskItem> AllProjectsFilter::refreshRecipe()
{
return Sync([this] { invalidateCache(); return true; });
}
} // ProjectExplorer::Internal

View File

@@ -5,8 +5,6 @@
#include <coreplugin/locator/basefilefilter.h>
#include <QFutureInterface>
namespace ProjectExplorer {
namespace Internal {
@@ -16,11 +14,11 @@ class AllProjectsFilter : public Core::BaseFileFilter
public:
AllProjectsFilter();
void refresh(QFutureInterface<void> &future) override;
void prepareSearch(const QString &entry) override;
private:
void markFilesAsOutOfDate();
void invalidateCache();
std::optional<Utils::Tasking::TaskItem> refreshRecipe() override;
};
} // namespace Internal

View File

@@ -8,6 +8,7 @@
#include "projecttree.h"
#include <utils/algorithm.h>
#include <utils/tasktree.h>
using namespace Core;
using namespace ProjectExplorer;
@@ -28,11 +29,6 @@ CurrentProjectFilter::CurrentProjectFilter()
this, &CurrentProjectFilter::currentProjectChanged);
}
void CurrentProjectFilter::markFilesAsOutOfDate()
{
setFileIterator(nullptr);
}
void CurrentProjectFilter::prepareSearch(const QString &entry)
{
Q_UNUSED(entry)
@@ -52,19 +48,24 @@ void CurrentProjectFilter::currentProjectChanged()
return;
if (m_project)
disconnect(m_project, &Project::fileListChanged,
this, &CurrentProjectFilter::markFilesAsOutOfDate);
this, &CurrentProjectFilter::invalidateCache);
if (project)
connect(project, &Project::fileListChanged,
this, &CurrentProjectFilter::markFilesAsOutOfDate);
this, &CurrentProjectFilter::invalidateCache);
m_project = project;
markFilesAsOutOfDate();
invalidateCache();
}
void CurrentProjectFilter::refresh(QFutureInterface<void> &future)
void CurrentProjectFilter::invalidateCache()
{
Q_UNUSED(future)
QMetaObject::invokeMethod(this, &CurrentProjectFilter::markFilesAsOutOfDate,
Qt::QueuedConnection);
setFileIterator(nullptr);
}
using namespace Utils::Tasking;
std::optional<TaskItem> CurrentProjectFilter::refreshRecipe()
{
return Sync([this] { invalidateCache(); return true; });
}

View File

@@ -5,8 +5,6 @@
#include <coreplugin/locator/basefilefilter.h>
#include <QFutureInterface>
namespace ProjectExplorer {
class Project;
@@ -19,12 +17,12 @@ class CurrentProjectFilter : public Core::BaseFileFilter
public:
CurrentProjectFilter();
void refresh(QFutureInterface<void> &future) override;
void prepareSearch(const QString &entry) override;
private:
void currentProjectChanged();
void markFilesAsOutOfDate();
void invalidateCache();
std::optional<Utils::Tasking::TaskItem> refreshRecipe() override;
Project *m_project = nullptr;
};