From e78d6bd3c420347fee20641689ab8c61bfb3e10c Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Sat, 4 Nov 2023 23:51:17 +0100 Subject: [PATCH] TaskTree: Implement storage shadowing Make it possible to have the same storage instance placed in different groups. When the same storage is placed in two nested groups, implement storage shadowing so that when the inner group is activated it activates only the innermost storage and shadows any possible the same storages in parent groups. Keep placing the same storage twice in one group forbidden. This functionality is required to implement the task tree loops (see 3rd point in the master task below). This addresses the 23th point in the master task below. Task-number: QTCREATORBUG-28741 Change-Id: Iba00bc32319430136a794974c14a1ab65272eaa9 Reviewed-by: Reviewed-by: hjk --- src/libs/solutions/tasking/tasktree.cpp | 69 +++++++++-------- src/libs/solutions/tasking/tasktree.h | 1 + tests/auto/solutions/tasking/tst_tasking.cpp | 79 +++++++++++++++++++- 3 files changed, 116 insertions(+), 33 deletions(-) diff --git a/src/libs/solutions/tasking/tasktree.cpp b/src/libs/solutions/tasking/tasktree.cpp index 37bc8385e21..84e6f544553 100644 --- a/src/libs/solutions/tasking/tasktree.cpp +++ b/src/libs/solutions/tasking/tasktree.cpp @@ -865,6 +865,11 @@ void *TreeStorageBase::activeStorageVoid() const return it.value(); } +int TreeStorageBase::activeStorageId() const +{ + return m_storageData->m_activeStorage; +} + int TreeStorageBase::createStorage() const { QTC_ASSERT(m_storageData->m_constructor, return 0); // TODO: add isValid()? @@ -895,6 +900,7 @@ void TreeStorageBase::activateStorage(int id) const return; } QTC_ASSERT(m_storageData->m_activeStorage == 0, return); + // TODO: Unneeded check? OTOH, it's quite important... const auto it = m_storageData->m_storageHash.find(id); QTC_ASSERT(it != m_storageData->m_storageHash.end(), return); m_storageData->m_activeStorage = id; @@ -946,7 +952,15 @@ void GroupItem::addChildren(const QList &children) m_children.append(child); break; case Type::Storage: - m_storageList.append(child.m_storageList); + // Check for duplicates, as can't have the same storage twice on the same level. + for (const TreeStorageBase &storage : child.m_storageList) { + if (m_storageList.contains(storage)) { + QTC_ASSERT(false, qWarning("Can't add the same storage into one Group twice, " + "skipping...")); + continue; + } + m_storageList.append(storage); + } break; } } @@ -985,7 +999,6 @@ public: void emitStartedAndProgress(); void emitProgress(); void emitDone(DoneWith result); - QList addStorages(const QList &storages); void callSetupHandler(TreeStorageBase storage, int storageId) { callStorageHandler(storage, storageId, &StorageHandler::m_setupHandler); } @@ -1151,45 +1164,33 @@ void TaskTreePrivate::emitDone(DoneWith result) emit q->done(result); } -QList TaskTreePrivate::addStorages(const QList &storages) -{ - QList addedStorages; - for (const TreeStorageBase &storage : storages) { - QTC_ASSERT(!m_storages.contains(storage), qWarning("Can't add the same storage into " - "one TaskTree twice, skipping..."); continue); - addedStorages << storage; - m_storages << storage; - } - return addedStorages; -} - class ExecutionContextActivator { public: - ExecutionContextActivator(TaskContainer *container) - : m_container(container) { activateContext(m_container); } - ~ExecutionContextActivator() { deactivateContext(m_container); } + ExecutionContextActivator(TaskContainer *container) { activateContext(container); } + ~ExecutionContextActivator() { + for (int i = m_activeStorages.size() - 1; i >= 0; --i) // iterate in reverse order + m_activeStorages[i].activateStorage(0); + } private: - static void activateContext(TaskContainer *container) + void activateContext(TaskContainer *container) { QTC_ASSERT(container && container->isRunning(), return); const TaskContainer::ConstData &constData = container->m_constData; + for (int i = 0; i < constData.m_storageList.size(); ++i) { + const TreeStorageBase &storage = constData.m_storageList[i]; + if (storage.activeStorageId()) + continue; // Storage shadowing: The storage is already active, skipping it... + m_activeStorages.append(storage); + storage.activateStorage(container->m_runtimeData->m_storageIdList.value(i)); + } + // Go to the parent after activating this storages so that storage shadowing works + // in the direction from child to parent root. if (constData.m_parentContainer) activateContext(constData.m_parentContainer); - for (int i = 0; i < constData.m_storageList.size(); ++i) - constData.m_storageList[i].activateStorage(container->m_runtimeData->m_storageIdList.value(i)); } - static void deactivateContext(TaskContainer *container) - { - QTC_ASSERT(container && container->isRunning(), return); - const TaskContainer::ConstData &constData = container->m_constData; - for (int i = constData.m_storageList.size() - 1; i >= 0; --i) // iterate in reverse order - constData.m_storageList[i].activateStorage(0); - if (constData.m_parentContainer) - deactivateContext(constData.m_parentContainer); - } - TaskContainer *m_container = nullptr; + QList m_activeStorages; }; template addStorages(task.m_storageList)) + , m_storageList(task.m_storageList) , m_children(createChildren(taskTreePrivate, thisContainer, task.m_children)) , m_taskCount(std::accumulate(m_children.cbegin(), m_children.cend(), 0, [](int r, TaskNode *n) { return r + n->taskCount(); })) -{} +{ + for (const TreeStorageBase &storage : m_storageList) + m_taskTreePrivate->m_storages << storage; +} QList TaskContainer::RuntimeData::createStorages(const TaskContainer::ConstData &constData) { @@ -2170,6 +2174,7 @@ void TaskTree::setRecipe(const Group &recipe) QTC_ASSERT(!isRunning(), qWarning("The TaskTree is already running, ignoring..."); return); QTC_ASSERT(!d->m_guard.isLocked(), qWarning("The setRecipe() is called from one of the" "TaskTree handlers, ignoring..."); return); + // TODO: Should we clear the m_storageHandlers, too? d->m_storages.clear(); d->m_root.reset(new TaskNode(d, recipe, nullptr)); } diff --git a/src/libs/solutions/tasking/tasktree.h b/src/libs/solutions/tasking/tasktree.h index d6375023b9f..a575d7c10c9 100644 --- a/src/libs/solutions/tasking/tasktree.h +++ b/src/libs/solutions/tasking/tasktree.h @@ -52,6 +52,7 @@ private: TreeStorageBase(StorageConstructor ctor, StorageDestructor dtor); void *activeStorageVoid() const; + int activeStorageId() const; int createStorage() const; void deleteStorage(int id) const; diff --git a/tests/auto/solutions/tasking/tst_tasking.cpp b/tests/auto/solutions/tasking/tst_tasking.cpp index ae0b3d63329..994d3d54967 100644 --- a/tests/auto/solutions/tasking/tst_tasking.cpp +++ b/tests/auto/solutions/tasking/tst_tasking.cpp @@ -4,6 +4,7 @@ #include #include +#include using namespace Tasking; @@ -39,7 +40,8 @@ enum class Handler { TweakDoneToError, Sync, BarrierAdvance, - Timeout + Timeout, + Storage }; Q_ENUM_NS(Handler); @@ -2425,6 +2427,81 @@ void tst_Tasking::testTree_data() QTest::newRow("GroupDoneWithTimeoutHandler") << TestData{storage, root4, doneLog, 2, OnDone::Success}; } + + { + // This test check if storage shadowing works OK. + + // This helper storage collect the pointers to storages created by shadowedStorage. + const TreeStorage> helperStorage; // One instance in this test. + // This storage is repeated in nested groups, the innermost storage will shadow outer ones. + const TreeStorage shadowedStorage; // Three instances in this test. + + const auto groupSetupWithStorage = [storage, helperStorage, shadowedStorage](int taskId) { + return onGroupSetup([storage, helperStorage, shadowedStorage, taskId] { + storage->m_log.append({taskId, Handler::GroupSetup}); + helperStorage->insert(taskId, shadowedStorage.activeStorage()); + *shadowedStorage = taskId; + }); + }; + const auto groupDoneWithStorage = [storage, helperStorage, shadowedStorage](int taskId) { + return onGroupDone([storage, helperStorage, shadowedStorage, taskId](DoneWith result) { + storage->m_log.append({taskId, resultToGroupHandler(result)}); + auto it = helperStorage->find(taskId); + if (it == helperStorage->end()) { + qWarning() << "The helperStorage is missing the shadowedStorage."; + return; + } else if (*it != shadowedStorage.activeStorage()) { + qWarning() << "Wrong active instance of the shadowedStorage."; + return; + } else if (**it != taskId) { + qWarning() << "Wrong data of the active instance of the shadowedStorage."; + return; + } + helperStorage->erase(it); + storage->m_log.append({*shadowedStorage, Handler::Storage}); + }); + }; + + const Group root { + Storage(storage), + Storage(helperStorage), + Storage(shadowedStorage), + groupSetupWithStorage(1), + Group { + Storage(shadowedStorage), + groupSetupWithStorage(2), + Group { + Storage(shadowedStorage), + groupSetupWithStorage(3), + groupDoneWithStorage(3) + }, + Group { + Storage(shadowedStorage), + groupSetupWithStorage(4), + groupDoneWithStorage(4) + }, + groupDoneWithStorage(2) + }, + groupDoneWithStorage(1) + }; + + const Log log { + {1, Handler::GroupSetup}, + {2, Handler::GroupSetup}, + {3, Handler::GroupSetup}, + {3, Handler::GroupSuccess}, + {3, Handler::Storage}, + {4, Handler::GroupSetup}, + {4, Handler::GroupSuccess}, + {4, Handler::Storage}, + {2, Handler::GroupSuccess}, + {2, Handler::Storage}, + {1, Handler::GroupSuccess}, + {1, Handler::Storage}, + }; + + QTest::newRow("StorageShadowing") << TestData{storage, root, log, 0, OnDone::Success}; + } } void tst_Tasking::testTree()