From 4b27cd73ffa9f8e9589f4cffb26720750f1e3478 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 9 Mar 2018 13:36:43 -0800 Subject: [PATCH 1/3] LogHandler class stores regexps rather than always recreating General efficiency tweaks; move to C++11. --- libraries/shared/src/LogHandler.cpp | 64 +++++++++++------------------ libraries/shared/src/LogHandler.h | 22 ++++++---- 2 files changed, 40 insertions(+), 46 deletions(-) diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp index cb3c0d07b2..ee063bbd31 100644 --- a/libraries/shared/src/LogHandler.cpp +++ b/libraries/shared/src/LogHandler.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include #include @@ -92,12 +91,13 @@ void LogHandler::setShouldDisplayMilliseconds(bool shouldDisplayMilliseconds) { void LogHandler::flushRepeatedMessages() { QMutexLocker lock(&_mutex); - QHash::iterator message = _repeatMessageCountHash.begin(); - while (message != _repeatMessageCountHash.end()) { + for(auto& message: _repeatedMessages) { - if (message.value() > 0) { + if (message->messageCount > 1) { QString repeatMessage = QString("%1 repeated log entries matching \"%2\" - Last entry: \"%3\"") - .arg(message.value()).arg(message.key()).arg(_lastRepeatedMessage.value(message.key())); + .arg(message->messageCount - 1) + .arg(message->regexp.pattern()) + .arg(message->lastMessage); QMessageLogContext emptyContext; lock.unlock(); @@ -105,8 +105,7 @@ void LogHandler::flushRepeatedMessages() { lock.relock(); } - _lastRepeatedMessage.remove(message.key()); - message = _repeatMessageCountHash.erase(message); + message->messageCount = 0; } } @@ -118,44 +117,25 @@ QString LogHandler::printMessage(LogMsgType type, const QMessageLogContext& cont if (type == LogDebug) { // for debug messages, check if this matches any of our regexes for repeated log messages - foreach(const QString& regexString, getInstance()._repeatedMessageRegexes) { - QRegExp repeatRegex(regexString); - if (repeatRegex.indexIn(message) != -1) { - - if (!_repeatMessageCountHash.contains(regexString)) { - // we have a match but didn't have this yet - output the first one - _repeatMessageCountHash[regexString] = 0; - - // break the foreach so we output the first match - break; - } else { - // we have a match - add 1 to the count of repeats for this message and set this as the last repeated message - _repeatMessageCountHash[regexString] += 1; - _lastRepeatedMessage[regexString] = message; - - // return out, we're not printing this one - return QString(); - } + for(auto& repeatRegex: _repeatedMessages) { + if (repeatRegex->regexp.indexIn(message) != -1) { + // If we've printed the first one then return out. + if (repeatRegex->messageCount++ == 0) break; + repeatRegex->lastMessage = message; + return QString(); } } } + if (type == LogDebug) { // see if this message is one we should only print once - foreach(const QString& regexString, getInstance()._onlyOnceMessageRegexes) { - QRegExp onlyOnceRegex(regexString); - if (onlyOnceRegex.indexIn(message) != -1) { - if (!_onlyOnceMessageCountHash.contains(message)) { - // we have a match and haven't yet printed this message. - _onlyOnceMessageCountHash[message] = 1; - // break the foreach so we output the first match - break; - } else { - // We've already printed this message, don't print it again. - return QString(); + for(auto& onceOnly : _onetimeMessages) { + if (onceOnly->regexp.indexIn(message) != -1) { + if (onceOnly->messageCount++ == 0) break; // we have a match and haven't yet printed this message. + else return QString(); // We've already printed this message, don't print it again. } } } - } // log prefix is in the following format // [TIMESTAMP] [DEBUG] [PID] [TID] [TARGET] logged string @@ -217,10 +197,16 @@ const QString& LogHandler::addRepeatedMessageRegex(const QString& regexString) { QMetaObject::invokeMethod(this, "setupRepeatedMessageFlusher"); QMutexLocker lock(&_mutex); - return *_repeatedMessageRegexes.insert(regexString); + std::unique_ptr<_RepeatedMessage> repeatRecord(new _RepeatedMessage()); + repeatRecord->regexp = QRegExp(regexString); + _repeatedMessages.insert(std::move(repeatRecord)); + return regexString; } const QString& LogHandler::addOnlyOnceMessageRegex(const QString& regexString) { QMutexLocker lock(&_mutex); - return *_onlyOnceMessageRegexes.insert(regexString); + std::unique_ptr<_OnceOnlyMessage> onetimeMessage(new _OnceOnlyMessage()); + onetimeMessage->regexp = QRegExp(regexString); + _onetimeMessages.insert(std::move(onetimeMessage)); + return regexString; } diff --git a/libraries/shared/src/LogHandler.h b/libraries/shared/src/LogHandler.h index ea961a8d4c..bbf6d62f03 100644 --- a/libraries/shared/src/LogHandler.h +++ b/libraries/shared/src/LogHandler.h @@ -13,11 +13,12 @@ #ifndef hifi_LogHandler_h #define hifi_LogHandler_h -#include #include -#include #include +#include #include +#include +#include const int VERBOSE_LOG_INTERVAL_SECONDS = 5; @@ -66,12 +67,19 @@ private: bool _shouldOutputProcessID { false }; bool _shouldOutputThreadID { false }; bool _shouldDisplayMilliseconds { false }; - QSet _repeatedMessageRegexes; - QHash _repeatMessageCountHash; - QHash _lastRepeatedMessage; - QSet _onlyOnceMessageRegexes; - QHash _onlyOnceMessageCountHash; + struct _RepeatedMessage { + QRegExp regexp; + int messageCount { 0 }; + QString lastMessage; + }; + std::set> _repeatedMessages; + + struct _OnceOnlyMessage { + QRegExp regexp; + int messageCount { 0 }; + }; + std::set> _onetimeMessages; static QMutex _mutex; }; From d4cf22fbaf03b145e321bb73b174d100ff1a6204 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 9 Mar 2018 15:11:05 -0800 Subject: [PATCH 2/3] LogHandler class stores regexps - code tweaks Code standard changes from review. --- libraries/shared/src/LogHandler.cpp | 20 ++++++++++++++------ libraries/shared/src/LogHandler.h | 18 +++++++++--------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp index ee063bbd31..88110da95b 100644 --- a/libraries/shared/src/LogHandler.cpp +++ b/libraries/shared/src/LogHandler.cpp @@ -117,10 +117,12 @@ QString LogHandler::printMessage(LogMsgType type, const QMessageLogContext& cont if (type == LogDebug) { // for debug messages, check if this matches any of our regexes for repeated log messages - for(auto& repeatRegex: _repeatedMessages) { + for(auto& repeatRegex : _repeatedMessages) { if (repeatRegex->regexp.indexIn(message) != -1) { // If we've printed the first one then return out. - if (repeatRegex->messageCount++ == 0) break; + if (repeatRegex->messageCount++ == 0) { + break; + } repeatRegex->lastMessage = message; return QString(); } @@ -131,11 +133,17 @@ QString LogHandler::printMessage(LogMsgType type, const QMessageLogContext& cont // see if this message is one we should only print once for(auto& onceOnly : _onetimeMessages) { if (onceOnly->regexp.indexIn(message) != -1) { - if (onceOnly->messageCount++ == 0) break; // we have a match and haven't yet printed this message. - else return QString(); // We've already printed this message, don't print it again. + if (onceOnly->messageCount++ == 0) { + // we have a match and haven't yet printed this message. + break; + } + else { + // We've already printed this message, don't print it again. + return QString(); } } } + } // log prefix is in the following format // [TIMESTAMP] [DEBUG] [PID] [TID] [TARGET] logged string @@ -197,7 +205,7 @@ const QString& LogHandler::addRepeatedMessageRegex(const QString& regexString) { QMetaObject::invokeMethod(this, "setupRepeatedMessageFlusher"); QMutexLocker lock(&_mutex); - std::unique_ptr<_RepeatedMessage> repeatRecord(new _RepeatedMessage()); + std::unique_ptr repeatRecord(new RepeatedMessage()); repeatRecord->regexp = QRegExp(regexString); _repeatedMessages.insert(std::move(repeatRecord)); return regexString; @@ -205,7 +213,7 @@ const QString& LogHandler::addRepeatedMessageRegex(const QString& regexString) { const QString& LogHandler::addOnlyOnceMessageRegex(const QString& regexString) { QMutexLocker lock(&_mutex); - std::unique_ptr<_OnceOnlyMessage> onetimeMessage(new _OnceOnlyMessage()); + std::unique_ptr onetimeMessage(new OnceOnlyMessage()); onetimeMessage->regexp = QRegExp(regexString); _onetimeMessages.insert(std::move(onetimeMessage)); return regexString; diff --git a/libraries/shared/src/LogHandler.h b/libraries/shared/src/LogHandler.h index bbf6d62f03..fbf7bddaab 100644 --- a/libraries/shared/src/LogHandler.h +++ b/libraries/shared/src/LogHandler.h @@ -68,18 +68,18 @@ private: bool _shouldOutputThreadID { false }; bool _shouldDisplayMilliseconds { false }; - struct _RepeatedMessage { - QRegExp regexp; - int messageCount { 0 }; - QString lastMessage; + struct RepeatedMessage { + QRegExp regexp; + int messageCount { 0 }; + QString lastMessage; }; - std::set> _repeatedMessages; + std::set> _repeatedMessages; - struct _OnceOnlyMessage { - QRegExp regexp; - int messageCount { 0 }; + struct OnceOnlyMessage { + QRegExp regexp; + int messageCount { 0 }; }; - std::set> _onetimeMessages; + std::set> _onetimeMessages; static QMutex _mutex; }; From cc166dbece16be8b9d92311a0e6c3c53045b984c Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Fri, 9 Mar 2018 15:44:56 -0800 Subject: [PATCH 3/3] LogHandler regexps - use vector of values --- libraries/shared/src/LogHandler.cpp | 39 ++++++++++++++--------------- libraries/shared/src/LogHandler.h | 6 ++--- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp index 88110da95b..49927a325b 100644 --- a/libraries/shared/src/LogHandler.cpp +++ b/libraries/shared/src/LogHandler.cpp @@ -93,11 +93,11 @@ void LogHandler::flushRepeatedMessages() { QMutexLocker lock(&_mutex); for(auto& message: _repeatedMessages) { - if (message->messageCount > 1) { + if (message.messageCount > 1) { QString repeatMessage = QString("%1 repeated log entries matching \"%2\" - Last entry: \"%3\"") - .arg(message->messageCount - 1) - .arg(message->regexp.pattern()) - .arg(message->lastMessage); + .arg(message.messageCount - 1) + .arg(message.regexp.pattern()) + .arg(message.lastMessage); QMessageLogContext emptyContext; lock.unlock(); @@ -105,7 +105,7 @@ void LogHandler::flushRepeatedMessages() { lock.relock(); } - message->messageCount = 0; + message.messageCount = 0; } } @@ -117,13 +117,13 @@ QString LogHandler::printMessage(LogMsgType type, const QMessageLogContext& cont if (type == LogDebug) { // for debug messages, check if this matches any of our regexes for repeated log messages - for(auto& repeatRegex : _repeatedMessages) { - if (repeatRegex->regexp.indexIn(message) != -1) { + for (auto& repeatRegex : _repeatedMessages) { + if (repeatRegex.regexp.indexIn(message) != -1) { // If we've printed the first one then return out. - if (repeatRegex->messageCount++ == 0) { + if (repeatRegex.messageCount++ == 0) { break; } - repeatRegex->lastMessage = message; + repeatRegex.lastMessage = message; return QString(); } } @@ -131,13 +131,12 @@ QString LogHandler::printMessage(LogMsgType type, const QMessageLogContext& cont if (type == LogDebug) { // see if this message is one we should only print once - for(auto& onceOnly : _onetimeMessages) { - if (onceOnly->regexp.indexIn(message) != -1) { - if (onceOnly->messageCount++ == 0) { + for (auto& onceOnly : _onetimeMessages) { + if (onceOnly.regexp.indexIn(message) != -1) { + if (onceOnly.messageCount++ == 0) { // we have a match and haven't yet printed this message. break; - } - else { + } else { // We've already printed this message, don't print it again. return QString(); } @@ -205,16 +204,16 @@ const QString& LogHandler::addRepeatedMessageRegex(const QString& regexString) { QMetaObject::invokeMethod(this, "setupRepeatedMessageFlusher"); QMutexLocker lock(&_mutex); - std::unique_ptr repeatRecord(new RepeatedMessage()); - repeatRecord->regexp = QRegExp(regexString); - _repeatedMessages.insert(std::move(repeatRecord)); + RepeatedMessage repeatRecord; + repeatRecord.regexp = QRegExp(regexString); + _repeatedMessages.push_back(repeatRecord); return regexString; } const QString& LogHandler::addOnlyOnceMessageRegex(const QString& regexString) { QMutexLocker lock(&_mutex); - std::unique_ptr onetimeMessage(new OnceOnlyMessage()); - onetimeMessage->regexp = QRegExp(regexString); - _onetimeMessages.insert(std::move(onetimeMessage)); + OnceOnlyMessage onetimeMessage; + onetimeMessage.regexp = QRegExp(regexString); + _onetimeMessages.push_back(onetimeMessage); return regexString; } diff --git a/libraries/shared/src/LogHandler.h b/libraries/shared/src/LogHandler.h index fbf7bddaab..2e64f16c1e 100644 --- a/libraries/shared/src/LogHandler.h +++ b/libraries/shared/src/LogHandler.h @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include const int VERBOSE_LOG_INTERVAL_SECONDS = 5; @@ -73,13 +73,13 @@ private: int messageCount { 0 }; QString lastMessage; }; - std::set> _repeatedMessages; + std::vector _repeatedMessages; struct OnceOnlyMessage { QRegExp regexp; int messageCount { 0 }; }; - std::set> _onetimeMessages; + std::vector _onetimeMessages; static QMutex _mutex; };