Added InstanceId to uniquely identify concurrent messages, transactions,
etc.
Use InstanceId for async job and calls identification.
Side-effect: removes inconsistent prefixes for job debugging: ecapxN,
icapxN, asyncN are now all jobN, simplifying searching and processing
debugging logs.
-----------
We have a few classes that use mostly unique identifiers for debugging.
More are coming, including MemBlob from the StringNG branch and IPC
messages used by SMP code. The classes I know about benefit from having
sequentially increasing IDs, specific to the class. For example, it is
very convenient to have async jobs numbered 1,2,3,4,... because it often
preserves the job number during repetitive debugging and also shows the
relative "distance" between the jobs.
The first attached patch adds a InstanceId class that encapsulates the
identifier maintenance logic. The second shows how the new class can be
used in the existing code.
Beside the general review, here are a few specific discussion points:
We can force hex formatting for printed ID values. This will shorten
printed IDs, but will also make them a little bit more difficult to
find/see in the logs. Please let me know whether you think we should use
hex.
We can also move custom "created/destructed" debug messages to
InstanceId, but I am not sure that is a good idea because it will either
remove constructor parameters from those lines or will force us to use
two debugging lines per constructor (one standard and one custom). I
think it is better to add global functions to standardize output prefix
than ask InstanceId to emit "created/destructed" debug messages. What do
you think?
Currently, InstanceId uses "int" to store ID values. I am not aware of
any classes that need unique IDs with storage types other than int
(e.g., to save space or further minimize wrapping). If there are such
classes, we need to make the storage type into a second template
parameter. Please let me know if you know of such classes.
Thank you,
Alex.
Added InstanceId to uniquely identify concurrent messages, transactions, etc.
=== added file 'src/base/InstanceId.h'
--- src/base/InstanceId.h 1970-01-01 00:00:00 +0000
+++ src/base/InstanceId.h 2010-09-29 16:43:27 +0000
@@ -0,0 +1,60 @@
+#ifndef SQUID_BASE_INSTANCE_ID_H
+#define SQUID_BASE_INSTANCE_ID_H
+
+#include "config.h"
+#include <iosfwd>
+
+/** Identifier for class instances
+ * - unique IDs for a large number of concurrent instances, but may wrap;
+ * - useful for debugging and insecure request/response matching;
+ * - sequential IDs within a class except when wrapping;
+ * - always positive IDs.
+ * \todo: add storage type parameter to support configurable Value types?
+ * \todo: add creation/destruction debugging?
+ */
+template <class Class>
+class InstanceId
+{
+public:
+ typedef unsigned int Value; ///< id storage type; \todo: parameterize?
+
+ InstanceId(): value(++Last ? Last : ++Last) {}
+
+ operator Value() const { return value; }
+ bool operator ==(const InstanceId &o) const { return value == o.value; }
+ bool operator !=(const InstanceId &o) const { return !(*this == o); }
+
+ /// prints Prefix followed by ID value; \todo: use HEX for value printing?
+ std::ostream &print(std::ostream &os) const;
+
+public:
+ static const char *Prefix; ///< Class shorthand string for debugging
+ const Value value; ///< instance identifier
+
+private:
+ InstanceId(const InstanceId& right); ///< not implemented; IDs are unique
+ InstanceId& operator=(const InstanceId &right); ///< not implemented
+
+private:
+ static Value Last; ///< the last used ID value
+};
+
+/// convenience macro to instantiate Class-specific stuff in .cc files
+#define InstanceIdDefinitions(Class, prefix) \
+ template<> std::ostream & \
+ InstanceId<Class>::print(std::ostream &os) const { \
+ return os << Prefix << value; \
+ } \
+ template<> const char *InstanceId<Class>::Prefix = prefix; \
+ template<> InstanceId<Class>::Value InstanceId<Class>::Last = 0
+
+
+/// print the id
+template <class Class>
+inline
+std::ostream &operator <<(std::ostream &os, const InstanceId<Class> &id) {
+ return id.print(os);
+}
+
+
+#endif /* SQUID_BASE_INSTANCE_ID_H */
=== modified file 'src/base/Makefile.am'
--- src/base/Makefile.am 2010-08-23 23:15:26 +0000
+++ src/base/Makefile.am 2010-09-29 16:15:11 +0000
@@ -13,5 +13,6 @@
AsyncCallQueue.cc \
AsyncCallQueue.h \
CbcPointer.h \
+ InstanceId.h \
TextException.cc \
TextException.h
Use InstanceId for async job and calls identification.
Side-effect: removes inconsistent prefixes for job debugging: ecapxN, icapxN,
asyncN are now all jobN, simplifying searching and processing debugging logs.
=== modified file 'src/adaptation/ecap/XactionRep.cc'
--- src/adaptation/ecap/XactionRep.cc 2010-08-29 21:50:09 +0000
+++ src/adaptation/ecap/XactionRep.cc 2010-09-29 18:14:05 +0000
@@ -446,7 +446,7 @@
buf.append(" A.", 3);
}
- buf.Printf(" ecapx%d]", id);
+ buf.Printf(" %s%u]", id.Prefix, id.value);
buf.terminate();
=== modified file 'src/adaptation/icap/Xaction.cc'
--- src/adaptation/icap/Xaction.cc 2010-09-13 00:19:25 +0000
+++ src/adaptation/icap/Xaction.cc 2010-09-29 18:14:27 +0000
@@ -538,7 +538,7 @@
buf.append("/", 1);
fillDoneStatus(buf);
- buf.Printf(" icapx%d]", id);
+ buf.Printf(" %s%u]", id.Prefix, id.value);
buf.terminate();
=== modified file 'src/base/AsyncCall.cc'
--- src/base/AsyncCall.cc 2009-04-09 22:31:13 +0000
+++ src/base/AsyncCall.cc 2010-09-29 16:40:44 +0000
@@ -7,17 +7,17 @@
#include "base/AsyncCallQueue.h"
#include "cbdata.h"
-unsigned int AsyncCall::TheLastId = 0;
+InstanceIdDefinitions(AsyncCall, "call");
/* AsyncCall */
AsyncCall::AsyncCall(int aDebugSection, int aDebugLevel,
const char *aName): name(aName), debugSection(aDebugSection),
- debugLevel(aDebugLevel), id(++TheLastId), theNext(0), isCanceled(NULL)
+ debugLevel(aDebugLevel), theNext(0), isCanceled(NULL)
{
debugs(debugSection, debugLevel, "The AsyncCall " << name << " constructed, this=" << this <<
- " [call" << id << ']');
+ " [" << id << ']');
}
AsyncCall::~AsyncCall()
@@ -29,7 +29,7 @@
AsyncCall::make()
{
debugs(debugSection, debugLevel, HERE << "make call " << name <<
- " [call"<< id << ']');
+ " [" << id << ']');
if (canFire()) {
fire();
return;
@@ -39,7 +39,7 @@
isCanceled = "unknown reason";
debugs(debugSection, debugLevel, HERE << "will not call " << name <<
- " [call"<< id << ']' << " because of " << isCanceled);
+ " [" << id << ']' << " because of " << isCanceled);
}
bool
@@ -47,7 +47,7 @@
{
if (isCanceled)
debugs(debugSection, debugLevel, HERE << "will not call " << name <<
- " [call"<< id << ']' << " also because " << reason);
+ " [" << id << ']' << " also because " << reason);
isCanceled = reason;
return false;
}
@@ -73,7 +73,7 @@
ScheduleCall(const char *fileName, int fileLine, AsyncCall::Pointer &call)
{
debugs(call->debugSection, call->debugLevel, fileName << "(" << fileLine <<
- ") will call " << *call << " [call"<< call->id << ']' );
+ ") will call " << *call << " [" << call->id << ']' );
AsyncCallQueue::Instance().schedule(call);
return true;
}
=== modified file 'src/base/AsyncCall.h'
--- src/base/AsyncCall.h 2009-04-09 22:31:13 +0000
+++ src/base/AsyncCall.h 2010-09-29 16:13:21 +0000
@@ -6,6 +6,7 @@
#define SQUID_ASYNCCALL_H
//#include "cbdata.h"
+#include "base/InstanceId.h"
#include "event.h"
//#include "TextException.h"
@@ -69,7 +70,7 @@
const char *const name;
const int debugSection;
const int debugLevel;
- const unsigned int id;
+ const InstanceId<AsyncCall> id;
protected:
virtual bool canFire();
@@ -80,7 +81,6 @@
private:
const char *isCanceled; // set to the cancelation reason by cancel()
- static unsigned int TheLastId;
};
inline
=== modified file 'src/base/AsyncJob.cc'
--- src/base/AsyncJob.cc 2010-09-12 00:10:47 +0000
+++ src/base/AsyncJob.cc 2010-09-29 18:15:38 +0000
@@ -10,8 +10,7 @@
#include "cbdata.h"
#include "MemBuf.h"
-
-unsigned int AsyncJob::TheLastId = 0;
+InstanceIdDefinitions(AsyncJob, "job");
AsyncJob::Pointer AsyncJob::Start(AsyncJob *j)
{
@@ -20,16 +19,16 @@
return job;
}
-AsyncJob::AsyncJob(const char *aTypeName): typeName(aTypeName), inCall(NULL), id(++TheLastId)
+AsyncJob::AsyncJob(const char *aTypeName): typeName(aTypeName), inCall(NULL)
{
debugs(93,5, "AsyncJob constructed, this=" << this <<
- " type=" << typeName << " [job" << id << ']');
+ " type=" << typeName << " [" << id << ']');
}
AsyncJob::~AsyncJob()
{
debugs(93,5, "AsyncJob destructed, this=" << this <<
- " type=" << typeName << " [job" << id << ']');
+ " type=" << typeName << " [" << id << ']');
}
void AsyncJob::start()
@@ -156,7 +155,7 @@
buf.Printf("Stopped, reason:");
buf.Printf("%s",stopReason);
}
- buf.Printf(" job%d]", id);
+ buf.Printf(" %s%u]", id.Prefix, id.value);
buf.terminate();
return buf.content();
=== modified file 'src/base/AsyncJob.h'
--- src/base/AsyncJob.h 2010-08-23 23:15:26 +0000
+++ src/base/AsyncJob.h 2010-09-29 16:57:30 +0000
@@ -6,6 +6,7 @@
#define SQUID_ASYNC_JOB_H
#include "base/AsyncCall.h"
+#include "base/InstanceId.h"
template <class Cbc>
class CbcPointer;
@@ -63,10 +64,7 @@
const char *stopReason; ///< reason for forcing done() to be true
const char *typeName; ///< kid (leaf) class name, for debugging
AsyncCall::Pointer inCall; ///< the asynchronous call being handled, if any
- const unsigned int id; ///< unique ID across all strand jobs, unless wraps
-
-private:
- static unsigned int TheLastId; ///< makes job IDs unique until it wraps
+ const InstanceId<AsyncJob> id; ///< job identifier
};
#endif /* SQUID_ASYNC_JOB_H */
