From ed3347a89b76003be9d58071b0b74a76e397104c Mon Sep 17 00:00:00 2001
From: Simon Walton <simon@highfidelity.io>
Date: Wed, 4 Apr 2018 14:11:10 -0700
Subject: [PATCH] Repeated logging - reviewer fixes
---
libraries/audio/src/AudioRingBuffer.cpp | 23 +++++++++++++++----
libraries/fbx/src/FBXReader_Mesh.cpp | 6 +++--
.../src/ObjectConstraintBallSocket.cpp | 6 +++--
.../physics/src/ObjectConstraintConeTwist.cpp | 6 +++--
.../physics/src/ObjectConstraintHinge.cpp | 6 +++--
.../physics/src/ObjectConstraintSlider.cpp | 6 +++--
libraries/render/src/render/DrawTask.cpp | 15 ++++++++++--
libraries/shared/src/LogHandler.cpp | 22 +++++++++---------
libraries/shared/src/LogHandler.h | 9 +++++---
9 files changed, 69 insertions(+), 30 deletions(-)
diff --git a/libraries/audio/src/AudioRingBuffer.cpp b/libraries/audio/src/AudioRingBuffer.cpp
index 7b1f24e519..683211aac6 100644
--- a/libraries/audio/src/AudioRingBuffer.cpp
+++ b/libraries/audio/src/AudioRingBuffer.cpp
@@ -151,6 +151,11 @@ int AudioRingBufferTemplate<T>::appendData(char *data, int maxSize) {
return numReadSamples * SampleSize;
}
+namespace {
+ int repeatedOverflowMessageID = 0;
+ std::atomic<int> messageIDInit = 0;
+}
+
template <class T>
int AudioRingBufferTemplate<T>::writeData(const char* data, int maxSize) {
// only copy up to the number of samples we have capacity for
@@ -164,8 +169,10 @@ int AudioRingBufferTemplate<T>::writeData(const char* data, int maxSize) {
_nextOutput = shiftedPositionAccomodatingWrap(_nextOutput, samplesToDelete);
_overflowCount++;
- HIFI_FCDEBUG(audio(), RING_BUFFER_OVERFLOW_DEBUG);
- qPrintable(RING_BUFFER_OVERFLOW_DEBUG);
+ if (++messageIDInit == 1) {
+ repeatedOverflowMessageID = LogHandler::getInstance().newRepeatedMessageID();
+ }
+ HIFI_FCDEBUG_ID(audio(), repeatedOverflowMessageID, RING_BUFFER_OVERFLOW_DEBUG);
}
if (_endOfLastWrite + numWriteSamples > _buffer + _bufferLength) {
@@ -273,7 +280,11 @@ int AudioRingBufferTemplate<T>::writeSamples(ConstIterator source, int maxSample
int samplesToDelete = samplesToCopy - samplesRoomFor;
_nextOutput = shiftedPositionAccomodatingWrap(_nextOutput, samplesToDelete);
_overflowCount++;
- HIFI_FCDEBUG(audio(), RING_BUFFER_OVERFLOW_DEBUG);
+
+ if (++messageIDInit == 1) {
+ repeatedOverflowMessageID = LogHandler::getInstance().newRepeatedMessageID();
+ }
+ HIFI_FCDEBUG_ID(audio(), repeatedOverflowMessageID, RING_BUFFER_OVERFLOW_DEBUG);
}
Sample* bufferLast = _buffer + _bufferLength - 1;
@@ -295,7 +306,11 @@ int AudioRingBufferTemplate<T>::writeSamplesWithFade(ConstIterator source, int m
int samplesToDelete = samplesToCopy - samplesRoomFor;
_nextOutput = shiftedPositionAccomodatingWrap(_nextOutput, samplesToDelete);
_overflowCount++;
- HIFI_FCDEBUG(audio(), RING_BUFFER_OVERFLOW_DEBUG);
+
+ if (++messageIDInit == 1) {
+ repeatedOverflowMessageID = LogHandler::getInstance().newRepeatedMessageID();
+ }
+ HIFI_FCDEBUG_ID(audio(), repeatedOverflowMessageID, RING_BUFFER_OVERFLOW_DEBUG);
}
Sample* bufferLast = _buffer + _bufferLength - 1;
diff --git a/libraries/fbx/src/FBXReader_Mesh.cpp b/libraries/fbx/src/FBXReader_Mesh.cpp
index 2cb9d3ed9f..e8365e38b7 100644
--- a/libraries/fbx/src/FBXReader_Mesh.cpp
+++ b/libraries/fbx/src/FBXReader_Mesh.cpp
@@ -571,13 +571,15 @@ void FBXReader::buildModelMesh(FBXMesh& extractedMesh, const QString& url) {
totalSourceIndices += (part.quadTrianglesIndices.size() + part.triangleIndices.size());
}
+ static int repeatMessageID = LogHandler::getInstance().newRepeatedMessageID();
+
if (!totalSourceIndices) {
- HIFI_FCDEBUG(modelformat(), "buildModelMesh failed -- no indices, url = " << url);
+ HIFI_FCDEBUG_ID(modelformat(), repeatMessageID, "buildModelMesh failed -- no indices, url = " << url);
return;
}
if (extractedMesh.vertices.size() == 0) {
- HIFI_FCDEBUG(modelformat(), "buildModelMesh failed -- no vertices, url = " << url);
+ HIFI_FCDEBUG_ID(modelformat(), repeatMessageID, "buildModelMesh failed -- no vertices, url = " << url);
return;
}
diff --git a/libraries/physics/src/ObjectConstraintBallSocket.cpp b/libraries/physics/src/ObjectConstraintBallSocket.cpp
index 1f1cb2487d..4736f2c9e2 100644
--- a/libraries/physics/src/ObjectConstraintBallSocket.cpp
+++ b/libraries/physics/src/ObjectConstraintBallSocket.cpp
@@ -85,9 +85,11 @@ btTypedConstraint* ObjectConstraintBallSocket::getConstraint() {
return constraint;
}
+ static int repeatMessageID = LogHandler::getInstance().newRepeatedMessageID();
+
btRigidBody* rigidBodyA = getRigidBody();
if (!rigidBodyA) {
- HIFI_FCDEBUG(physics(), "ObjectConstraintBallSocket::getConstraint -- no rigidBodyA");
+ HIFI_FCDEBUG_ID(physics(), repeatMessageID, "ObjectConstraintBallSocket::getConstraint -- no rigidBodyA");
return nullptr;
}
@@ -96,7 +98,7 @@ btTypedConstraint* ObjectConstraintBallSocket::getConstraint() {
btRigidBody* rigidBodyB = getOtherRigidBody(otherEntityID);
if (!rigidBodyB) {
- HIFI_FCDEBUG(physics(), "ObjectConstraintBallSocket::getConstraint -- no rigidBodyB");
+ HIFI_FCDEBUG_ID(physics(), repeatMessageID, "ObjectConstraintBallSocket::getConstraint -- no rigidBodyB");
return nullptr;
}
diff --git a/libraries/physics/src/ObjectConstraintConeTwist.cpp b/libraries/physics/src/ObjectConstraintConeTwist.cpp
index 714fd662e1..47228c1c16 100644
--- a/libraries/physics/src/ObjectConstraintConeTwist.cpp
+++ b/libraries/physics/src/ObjectConstraintConeTwist.cpp
@@ -96,9 +96,11 @@ btTypedConstraint* ObjectConstraintConeTwist::getConstraint() {
return constraint;
}
+ static int repeatMessageID = LogHandler::getInstance().newRepeatedMessageID();
+
btRigidBody* rigidBodyA = getRigidBody();
if (!rigidBodyA) {
- HIFI_FCDEBUG(physics(), "ObjectConstraintConeTwist::getConstraint -- no rigidBodyA");
+ HIFI_FCDEBUG_ID(physics(), repeatMessageID, "ObjectConstraintConeTwist::getConstraint -- no rigidBodyA");
return nullptr;
}
@@ -127,7 +129,7 @@ btTypedConstraint* ObjectConstraintConeTwist::getConstraint() {
btRigidBody* rigidBodyB = getOtherRigidBody(otherEntityID);
if (!rigidBodyB) {
- HIFI_FCDEBUG(physics(), "ObjectConstraintConeTwist::getConstraint -- no rigidBodyB");
+ HIFI_FCDEBUG_ID(physics(), repeatMessageID, "ObjectConstraintConeTwist::getConstraint -- no rigidBodyB");
return nullptr;
}
diff --git a/libraries/physics/src/ObjectConstraintHinge.cpp b/libraries/physics/src/ObjectConstraintHinge.cpp
index 18014cddca..4793741391 100644
--- a/libraries/physics/src/ObjectConstraintHinge.cpp
+++ b/libraries/physics/src/ObjectConstraintHinge.cpp
@@ -94,10 +94,12 @@ btTypedConstraint* ObjectConstraintHinge::getConstraint() {
if (constraint) {
return constraint;
}
+
+ static int repeatMessageID = LogHandler::getInstance().newRepeatedMessageID();
btRigidBody* rigidBodyA = getRigidBody();
if (!rigidBodyA) {
- HIFI_FCDEBUG(physics(), "ObjectConstraintHinge::getConstraint -- no rigidBodyA");
+ HIFI_FCDEBUG_ID(physics(), repeatMessageID, "ObjectConstraintHinge::getConstraint -- no rigidBodyA");
return nullptr;
}
@@ -112,7 +114,7 @@ btTypedConstraint* ObjectConstraintHinge::getConstraint() {
// This hinge is between two entities... find the other rigid body.
btRigidBody* rigidBodyB = getOtherRigidBody(otherEntityID);
if (!rigidBodyB) {
- HIFI_FCDEBUG(physics(), "ObjectConstraintHinge::getConstraint -- no rigidBodyB");
+ HIFI_FCDEBUG_ID(physics(), repeatMessageID, "ObjectConstraintHinge::getConstraint -- no rigidBodyB");
return nullptr;
}
diff --git a/libraries/physics/src/ObjectConstraintSlider.cpp b/libraries/physics/src/ObjectConstraintSlider.cpp
index 9ae34e1124..da5bba7f4d 100644
--- a/libraries/physics/src/ObjectConstraintSlider.cpp
+++ b/libraries/physics/src/ObjectConstraintSlider.cpp
@@ -87,9 +87,11 @@ btTypedConstraint* ObjectConstraintSlider::getConstraint() {
return constraint;
}
+ static int repeatMessageID = LogHandler::getInstance().newRepeatedMessageID();
+
btRigidBody* rigidBodyA = getRigidBody();
if (!rigidBodyA) {
- HIFI_FCDEBUG(physics(), "ObjectConstraintSlider::getConstraint -- no rigidBodyA");
+ HIFI_FCDEBUG_ID(physics(), repeatMessageID, "ObjectConstraintSlider::getConstraint -- no rigidBodyA");
return nullptr;
}
@@ -118,7 +120,7 @@ btTypedConstraint* ObjectConstraintSlider::getConstraint() {
btRigidBody* rigidBodyB = getOtherRigidBody(otherEntityID);
if (!rigidBodyB) {
- HIFI_FCDEBUG(physics(), "ObjectConstraintSlider::getConstraint -- no rigidBodyB");
+ HIFI_FCDEBUG_ID(physics(), repeatMessageID, "ObjectConstraintSlider::getConstraint -- no rigidBodyB");
return nullptr;
}
diff --git a/libraries/render/src/render/DrawTask.cpp b/libraries/render/src/render/DrawTask.cpp
index 18335a1296..86a6dee145 100755
--- a/libraries/render/src/render/DrawTask.cpp
+++ b/libraries/render/src/render/DrawTask.cpp
@@ -41,6 +41,11 @@ void render::renderItems(const RenderContextPointer& renderContext, const ItemBo
}
}
+namespace {
+ int repeatedInvalidKeyMessageID = 0;
+ std::atomic<int> messageIDInit = 0;
+}
+
void renderShape(RenderArgs* args, const ShapePlumberPointer& shapeContext, const Item& item, const ShapeKey& globalKey) {
assert(item.getKey().isShape());
auto key = item.getShapeKey() | globalKey;
@@ -55,7 +60,10 @@ void renderShape(RenderArgs* args, const ShapePlumberPointer& shapeContext, cons
} else if (key.hasOwnPipeline()) {
item.render(args);
} else {
- HIFI_FCDEBUG(renderlogging(), "Item could not be rendered with invalid key" << key);
+ if (++messageIDInit == 1) {
+ repeatedInvalidKeyMessageID = LogHandler::getInstance().newRepeatedMessageID();
+ }
+ HIFI_FCDEBUG_ID(renderlogging(), repeatedInvalidKeyMessageID, "Item could not be rendered with invalid key" << key);
}
args->_itemShapeKey = 0;
}
@@ -106,7 +114,10 @@ void render::renderStateSortShapes(const RenderContextPointer& renderContext,
} else if (key.hasOwnPipeline()) {
ownPipelineBucket.push_back( std::make_tuple(item, key) );
} else {
- HIFI_FCDEBUG(renderlogging(), "Item could not be rendered with invalid key" << key);
+ if (++messageIDInit == 1) {
+ repeatedInvalidKeyMessageID = LogHandler::getInstance().newRepeatedMessageID();
+ }
+ HIFI_FCDEBUG_ID(renderlogging(), repeatedInvalidKeyMessageID, "Item could not be rendered with invalid key" << key);
}
}
}
diff --git a/libraries/shared/src/LogHandler.cpp b/libraries/shared/src/LogHandler.cpp
index 06287e4d81..70e41b8304 100644
--- a/libraries/shared/src/LogHandler.cpp
+++ b/libraries/shared/src/LogHandler.cpp
@@ -92,15 +92,15 @@ void LogHandler::setShouldDisplayMilliseconds(bool shouldDisplayMilliseconds) {
void LogHandler::flushRepeatedMessages() {
QMutexLocker lock(&_mutex);
- // New repeat-supress scheme:
- for (int m = 0; m < (int)_repeatCounts.size(); ++m) {
- int repeatCount = _repeatCounts[m];
+ // New repeat-suppress scheme:
+ for (int m = 0; m < (int)_repeatedMessageRecords.size(); ++m) {
+ int repeatCount = _repeatedMessageRecords[m].repeatCount;
if (repeatCount > 1) {
QString repeatLogMessage = QString().setNum(repeatCount) + " repeated log entries - Last entry: \""
- + _repeatedMessageStrings[m] + "\"";
+ + _repeatedMessageRecords[m].repeatString + "\"";
printMessage(LogSuppressed, QMessageLogContext(), repeatLogMessage);
- _repeatCounts[m] = 0;
- _repeatedMessageStrings[m] = QString();
+ _repeatedMessageRecords[m].repeatCount = 0;
+ _repeatedMessageRecords[m].repeatString = QString();
}
}
}
@@ -193,8 +193,8 @@ int LogHandler::newRepeatedMessageID() {
QMutexLocker lock(&_mutex);
int newMessageId = _currentMessageID;
++_currentMessageID;
- _repeatCounts.push_back(0);
- _repeatedMessageStrings.resize(_currentMessageID);
+ RepeatedMessageRecord newRecord { 0 };
+ _repeatedMessageRecords.push_back(newRecord);
return newMessageId;
}
@@ -205,11 +205,11 @@ void LogHandler::printRepeatedMessage(int messageID, LogMsgType type, const QMes
return;
}
- if (_repeatCounts[messageID] == 0) {
+ if (_repeatedMessageRecords[messageID].repeatCount == 0) {
printMessage(type, context, message);
} else {
- _repeatedMessageStrings[messageID] = message;
+ _repeatedMessageRecords[messageID].repeatString = message;
}
- ++_repeatCounts[messageID];
+ ++_repeatedMessageRecords[messageID].repeatCount;
}
diff --git a/libraries/shared/src/LogHandler.h b/libraries/shared/src/LogHandler.h
index dfdfee6c3d..2b1f9c47aa 100644
--- a/libraries/shared/src/LogHandler.h
+++ b/libraries/shared/src/LogHandler.h
@@ -76,8 +76,11 @@ private:
std::vector<OnceOnlyMessage> _onetimeMessages;
int _currentMessageID { 0 };
- std::vector<int> _repeatCounts;
- std::vector<QString> _repeatedMessageStrings;
+ struct RepeatedMessageRecord {
+ int repeatCount;
+ QString repeatString;
+ };
+ std::vector<RepeatedMessageRecord> _repeatedMessageRecords;
static QMutex _mutex;
};
@@ -106,6 +109,6 @@ private:
} \
} while (false)
-#define HIFI_FDEBUG_ID(message) HIFI_FCDEBUG_ID((*QLoggingCategory::defaultCategory()), message)
+#define HIFI_FDEBUG_ID(messageID, message) HIFI_FCDEBUG_ID((*QLoggingCategory::defaultCategory()), messageID, message)
#endif // hifi_LogHandler_h