QLoggingRegistry: remove rules vector

It only contained a concatenation of the individual rule sets,
probably to fix their order in a central place, as well as
simplifying iteration in defaultCategoryFilter().

Fix these two issues differently, but introducing a RuleSet
enum that lists rule sets in the order in which they should
be applied by defaultCategoryFilter(), and turn individual
rule sets vectors into a C array of vectors.

This enables two nested loops in defaultCategoryFilter to
replace the one loop over 'rules'. Apart from building up
'rules' in updateRules(), this was the only access to that
member. That leaves updateRules() with just the task of
running defaultCategoryFilter() on the new rule sets.
Consequently, a call to updateRules() can now replace the
identical loop in installFilter().

Performance should not suffer. Iterating over a fixed-size
array of vectors is hardly any slower than iterating over
a single vector, and while the construction of 'rules'
was probably a one-off task in most programs, this way
of keeping the rules also saves memory because rules are
not kept in two different vectors.

It is also more maintainable, of course.

Change-Id: Ibc132d096c8137dd02b034752646212e51208637
Reviewed-by: Kai Koehne <kai.koehne@qt.io>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
Marc Mutz 2017-02-18 09:41:30 +01:00
parent 45104dff04
commit dfc2a4a537
3 changed files with 44 additions and 48 deletions

View File

@ -317,13 +317,12 @@ void QLoggingRegistry::init()
const QMutexLocker locker(&registryMutex);
envRules = std::move(er);
qtConfigRules = std::move(qr);
configRules = std::move(cr);
ruleSets[EnvironmentRules] = std::move(er);
ruleSets[QtConfigRules] = std::move(qr);
ruleSets[ConfigRules] = std::move(cr);
if (!envRules.isEmpty() || !qtConfigRules.isEmpty() || !configRules.isEmpty()) {
if (!ruleSets[EnvironmentRules].isEmpty() || !ruleSets[QtConfigRules].isEmpty() || !ruleSets[ConfigRules].isEmpty())
updateRules();
}
}
/*!
@ -367,7 +366,7 @@ void QLoggingRegistry::setApiRules(const QString &content)
const QMutexLocker locker(&registryMutex);
apiRules = parser.rules();
ruleSets[ApiRules] = parser.rules();
updateRules();
}
@ -380,13 +379,6 @@ void QLoggingRegistry::setApiRules(const QString &content)
*/
void QLoggingRegistry::updateRules()
{
rules.clear();
rules.reserve(qtConfigRules.size() + configRules.size() + apiRules.size() + envRules.size()),
rules += qtConfigRules;
rules += configRules;
rules += apiRules;
rules += envRules;
for (auto it = categories.keyBegin(), end = categories.keyEnd(); it != end; ++it)
(*categoryFilter)(*it);
}
@ -406,8 +398,7 @@ QLoggingRegistry::installFilter(QLoggingCategory::CategoryFilter filter)
QLoggingCategory::CategoryFilter old = categoryFilter;
categoryFilter = filter;
for (auto it = categories.keyBegin(), end = categories.keyEnd(); it != end; ++it)
(*categoryFilter)(*it);
updateRules();
return old;
}
@ -446,19 +437,22 @@ void QLoggingRegistry::defaultCategoryFilter(QLoggingCategory *cat)
}
QString categoryName = QLatin1String(cat->categoryName());
for (const QLoggingRule &item : reg->rules) {
int filterpass = item.pass(categoryName, QtDebugMsg);
if (filterpass != 0)
debug = (filterpass > 0);
filterpass = item.pass(categoryName, QtInfoMsg);
if (filterpass != 0)
info = (filterpass > 0);
filterpass = item.pass(categoryName, QtWarningMsg);
if (filterpass != 0)
warning = (filterpass > 0);
filterpass = item.pass(categoryName, QtCriticalMsg);
if (filterpass != 0)
critical = (filterpass > 0);
for (const auto &ruleSet : reg->ruleSets) {
for (const auto &rule : ruleSet) {
int filterpass = rule.pass(categoryName, QtDebugMsg);
if (filterpass != 0)
debug = (filterpass > 0);
filterpass = rule.pass(categoryName, QtInfoMsg);
if (filterpass != 0)
info = (filterpass > 0);
filterpass = rule.pass(categoryName, QtWarningMsg);
if (filterpass != 0)
warning = (filterpass > 0);
filterpass = rule.pass(categoryName, QtCriticalMsg);
if (filterpass != 0)
critical = (filterpass > 0);
}
}
cat->setEnabled(QtDebugMsg, debug);

View File

@ -130,14 +130,20 @@ private:
static void defaultCategoryFilter(QLoggingCategory *category);
enum RuleSet {
// sorted by order in which defaultCategoryFilter considers them:
QtConfigRules,
ConfigRules,
ApiRules,
EnvironmentRules,
NumRuleSets
};
QMutex registryMutex;
// protected by mutex:
QVector<QLoggingRule> qtConfigRules;
QVector<QLoggingRule> configRules;
QVector<QLoggingRule> envRules;
QVector<QLoggingRule> apiRules;
QVector<QLoggingRule> rules;
QVector<QLoggingRule> ruleSets[NumRuleSets];
QHash<QLoggingCategory*,QtMsgType> categories;
QLoggingCategory::CategoryFilter categoryFilter;

View File

@ -202,18 +202,15 @@ private slots:
QLoggingRegistry registry;
registry.init();
QCOMPARE(registry.apiRules.size(), 0);
QCOMPARE(registry.configRules.size(), 0);
QCOMPARE(registry.envRules.size(), 1);
QCOMPARE(registry.rules.size(), 1);
QCOMPARE(registry.ruleSets[QLoggingRegistry::ApiRules].size(), 0);
QCOMPARE(registry.ruleSets[QLoggingRegistry::ConfigRules].size(), 0);
QCOMPARE(registry.ruleSets[QLoggingRegistry::EnvironmentRules].size(), 1);
// check that QT_LOGGING_RULES take precedence
qputenv("QT_LOGGING_RULES", "Digia.*=true");
registry.init();
QCOMPARE(registry.envRules.size(), 2);
QCOMPARE(registry.envRules.at(1).enabled, true);
QCOMPARE(registry.rules.size(), 2);
QCOMPARE(registry.ruleSets[QLoggingRegistry::EnvironmentRules].size(), 2);
QCOMPARE(registry.ruleSets[QLoggingRegistry::EnvironmentRules].at(1).enabled, true);
}
void QLoggingRegistry_config()
@ -238,7 +235,7 @@ private slots:
QLoggingRegistry registry;
registry.init();
QCOMPARE(registry.configRules.size(), 1);
QCOMPARE(registry.ruleSets[QLoggingRegistry::ConfigRules].size(), 1);
// remove file again
QVERIFY(file.remove());
@ -260,10 +257,9 @@ private slots:
QLoggingRegistry *registry = QLoggingRegistry::instance();
// empty all rules , check default
registry->rules.clear();
registry->apiRules.clear();
registry->configRules.clear();
registry->envRules.clear();
registry->ruleSets[QLoggingRegistry::ApiRules].clear();
registry->ruleSets[QLoggingRegistry::ConfigRules].clear();
registry->ruleSets[QLoggingRegistry::EnvironmentRules].clear();
registry->updateRules();
QVERIFY(cat.isWarningEnabled());
@ -271,7 +267,7 @@ private slots:
// set Config rule
QLoggingSettingsParser parser;
parser.setContent("[Rules]\nDigia.*=false");
registry->configRules=parser.rules();
registry->ruleSets[QLoggingRegistry::ConfigRules] = parser.rules();
registry->updateRules();
QVERIFY(!cat.isWarningEnabled());
@ -283,7 +279,7 @@ private slots:
// set Env rule, should overwrite Config one
parser.setContent("Digia.*=false");
registry->envRules=parser.rules();
registry->ruleSets[QLoggingRegistry::EnvironmentRules] = parser.rules();
registry->updateRules();
QVERIFY(!cat.isWarningEnabled());