From 24b9f6ee06fd8e84b4904d7c9fadff9b4d188216 Mon Sep 17 00:00:00 2001 From: Jules Clero Date: Tue, 3 Mar 2015 11:22:14 +0100 Subject: [PATCH 1/2] Let a domain decide when he should be synchronized. Domains are browsed twice when applying configurations. The first one gather syncer of domains which are not "sequence aware" and launch a sync on them. The second one launch an apply without providing a syncer set. It lets sequence aware domains to synchronize if needed. This patch allows to browse domains only once. If a domain is sequence aware, it will be synchronized during configurations restauration. If not, the synchronisation will be delayed at the end of all applies. Sequence aware domains are now synchronized first. Signed-off-by: Jules Clero --- parameter/ConfigurableDomain.cpp | 22 +++++++--------------- parameter/ConfigurableDomain.h | 11 +++++++++-- parameter/ConfigurableDomains.cpp | 11 +---------- 3 files changed, 17 insertions(+), 27 deletions(-) diff --git a/parameter/ConfigurableDomain.cpp b/parameter/ConfigurableDomain.cpp index 35acb9caf..53081dba9 100644 --- a/parameter/ConfigurableDomain.cpp +++ b/parameter/ConfigurableDomain.cpp @@ -519,15 +519,10 @@ const CDomainConfiguration* CConfigurableDomain::getPendingConfiguration() const } // Configuration application if required -void CConfigurableDomain::apply(CParameterBlackboard* pParameterBlackboard, CSyncerSet* pSyncerSet, bool bForce) const +void CConfigurableDomain::apply(CParameterBlackboard* pParameterBlackboard, + CSyncerSet& syncerSet, + bool bForce) const { - // Apply configuration only if the blackboard will - // be synchronized either now or by syncerSet. - if(!pSyncerSet ^ _bSequenceAware) { - // The configuration can not be syncronised - return; - } - if (bForce) { // Force a configuration restore by forgetting about last applied configuration _pLastAppliedConfiguration = NULL; @@ -543,20 +538,17 @@ void CConfigurableDomain::apply(CParameterBlackboard* pParameterBlackboard, CSyn pApplicableDomainConfiguration->getName().c_str(), getName().c_str()); - // Check if we need to synchronize during restore - bool bSync = !pSyncerSet && _bSequenceAware; - - // Do the restore - pApplicableDomainConfiguration->restore(pParameterBlackboard, bSync, NULL); + // Do the restore, and synchronize if we are sequence aware + pApplicableDomainConfiguration->restore(pParameterBlackboard, _bSequenceAware, NULL); // Record last applied configuration _pLastAppliedConfiguration = pApplicableDomainConfiguration; // Check we need to provide syncer set to caller - if (pSyncerSet && !_bSequenceAware) { + if (!_bSequenceAware) { // Since we applied changes, add our own sync set to the given one - *pSyncerSet += _syncerSet; + syncerSet += _syncerSet; } } } diff --git a/parameter/ConfigurableDomain.h b/parameter/ConfigurableDomain.h index a29c1ba04..07fb05c7c 100644 --- a/parameter/ConfigurableDomain.h +++ b/parameter/ConfigurableDomain.h @@ -95,8 +95,15 @@ class CConfigurableDomain : public CBinarySerializableElement // Ensure validity on whole domain from main blackboard void validate(const CParameterBlackboard* pMainBlackboard); - // Configuration application if required - void apply(CParameterBlackboard* pParameterBlackboard, CSyncerSet* pSyncerSet, bool bForced) const; + /** Apply the configuration if required + * + * @param[in] pParameterBlackboard the blackboard to synchronize + * @param[in] syncerSet the set containing application syncers + * @param[in] bForced boolean used to force configuration application + */ + void apply(CParameterBlackboard* pParameterBlackboard, + CSyncerSet& syncerSet, + bool bForced) const; // Return applicable configuration validity for given configurable element bool isApplicableConfigurationValid(const CConfigurableElement* pConfigurableElement) const; diff --git a/parameter/ConfigurableDomains.cpp b/parameter/ConfigurableDomains.cpp index b77e2aad9..b39968e4c 100644 --- a/parameter/ConfigurableDomains.cpp +++ b/parameter/ConfigurableDomains.cpp @@ -83,19 +83,10 @@ void CConfigurableDomains::apply(CParameterBlackboard* pParameterBlackboard, CSy const CConfigurableDomain* pChildConfigurableDomain = static_cast(getChild(uiChild)); // Apply and collect syncers when relevant - pChildConfigurableDomain->apply(pParameterBlackboard, &syncerSet, bForce); + pChildConfigurableDomain->apply(pParameterBlackboard, syncerSet, bForce); } // Synchronize those collected syncers syncerSet.sync(*pParameterBlackboard, false, NULL); - - // Then deal with domains that need to synchronize along apply - for (uiChild = 0; uiChild < uiNbConfigurableDomains; uiChild++) { - - const CConfigurableDomain* pChildConfigurableDomain = static_cast(getChild(uiChild)); - - // Apply and synchronize when relevant - pChildConfigurableDomain->apply(pParameterBlackboard, NULL, bForce); - } } // From IXmlSource From ac259c3b0122436cdc50b11a0659e4c4b42261cd Mon Sep 17 00:00:00 2001 From: Jules Clero Date: Thu, 5 Mar 2015 17:57:01 +0100 Subject: [PATCH 2/2] Avoid redundant condition test _pLastAppliedConfiguration != NULL is tested twice as we know that pApplicableDomainConfiguration is not null. This patch avoids this double test and merge the two conditions. Signed-off-by: Jules Clero --- parameter/ConfigurableDomain.cpp | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/parameter/ConfigurableDomain.cpp b/parameter/ConfigurableDomain.cpp index 53081dba9..525e11e37 100644 --- a/parameter/ConfigurableDomain.cpp +++ b/parameter/ConfigurableDomain.cpp @@ -529,27 +529,25 @@ void CConfigurableDomain::apply(CParameterBlackboard* pParameterBlackboard, } const CDomainConfiguration* pApplicableDomainConfiguration = findApplicableDomainConfiguration(); - if (pApplicableDomainConfiguration) { - - // Check not the last one before applying - if (!_pLastAppliedConfiguration || _pLastAppliedConfiguration != pApplicableDomainConfiguration) { + // Check the last applied configuration is different from this one before apply + if (pApplicableDomainConfiguration != NULL && + _pLastAppliedConfiguration != pApplicableDomainConfiguration) { - log_info("Applying configuration \"%s\" from domain \"%s\"", - pApplicableDomainConfiguration->getName().c_str(), - getName().c_str()); + log_info("Applying configuration \"%s\" from domain \"%s\"", + pApplicableDomainConfiguration->getName().c_str(), + getName().c_str()); - // Do the restore, and synchronize if we are sequence aware - pApplicableDomainConfiguration->restore(pParameterBlackboard, _bSequenceAware, NULL); + // Do the restore, and synchronize if we are sequence aware + pApplicableDomainConfiguration->restore(pParameterBlackboard, _bSequenceAware, NULL); - // Record last applied configuration - _pLastAppliedConfiguration = pApplicableDomainConfiguration; + // Record last applied configuration + _pLastAppliedConfiguration = pApplicableDomainConfiguration; - // Check we need to provide syncer set to caller - if (!_bSequenceAware) { + // Check we need to provide syncer set to caller + if (!_bSequenceAware) { - // Since we applied changes, add our own sync set to the given one - syncerSet += _syncerSet; - } + // Since we applied changes, add our own sync set to the given one + syncerSet += _syncerSet; } } }