6 files changed, 87 insertions, 26 deletions
diff --git a/cpp/include/IceUtil/Cond.h b/cpp/include/IceUtil/Cond.h
index b3274c77464..6d3fabfdcff 100644
--- a/cpp/include/IceUtil/Cond.h
+++ b/cpp/include/IceUtil/Cond.h
@@ -175,6 +175,9 @@ private:
     }
  */
 
+    //
+    // TODO: Should not be inlined, not performance critical.
+    //
     void
     waitImpl(const RecMutex& mutex) const
     {
@@ -195,6 +198,9 @@ private:
 	}
     }
 
+    //
+    // TODO: Should not be inlined, not performance critical.
+    //
     void
     waitImpl(const Mutex& mutex) const
     {
@@ -215,6 +221,9 @@ private:
 	}
     }
     
+    //
+    // TODO: Should not be inlined, not performance critical.
+    //
     bool
     timedwaitImpl(const RecMutex& mutex, long msec) const
     {
@@ -236,6 +245,9 @@ private:
 	}
     }
 
+    //
+    // TODO: Should not be inlined, not performance critical.
+    //
     bool
     timedwaitImpl(const Mutex& mutex, long msec) const
     {
diff --git a/cpp/include/IceUtil/Config.h b/cpp/include/IceUtil/Config.h
index 8ae749e43e8..a9fb48327bc 100644
--- a/cpp/include/IceUtil/Config.h
+++ b/cpp/include/IceUtil/Config.h
@@ -82,6 +82,12 @@
 namespace IceUtil
 {
 
+//
+// TODO: Constructor and destructor should not be inlined, as they are
+// not performance critical.
+//
+// TODO: Naming conventions?
+//
 class noncopyable
 {
 protected:
diff --git a/cpp/include/IceUtil/Mutex.h b/cpp/include/IceUtil/Mutex.h
index 0c43398b279..d76fabe83b1 100644
--- a/cpp/include/IceUtil/Mutex.h
+++ b/cpp/include/IceUtil/Mutex.h
@@ -110,7 +110,14 @@ private:
 #endif
 };
 
+//
+// TODO: Check if all the functions below are really performance
+// critical. Those which are not performance critical shouldn't be
+// inlined.
+//
+
 #ifdef WIN32
+
 inline
 Mutex::Mutex()
 {
@@ -237,6 +244,7 @@ inline void
 Mutex::lock(LockState&) const
 {
 }
+
 #endif    
 
 } // End namespace IceUtil
diff --git a/cpp/include/IceUtil/OutputUtil.h b/cpp/include/IceUtil/OutputUtil.h
index 914c70b0da1..61ce7916949 100644
--- a/cpp/include/IceUtil/OutputUtil.h
+++ b/cpp/include/IceUtil/OutputUtil.h
@@ -36,8 +36,7 @@ public:
     OutputBase();
     OutputBase(std::ostream&);
     OutputBase(const char*);
-
-    virtual ~OutputBase() { }
+    virtual ~OutputBase();
 
     void setIndent(int); // What is the indent level?
     void setUseTab(bool); // Should we output tabs?
@@ -70,10 +69,14 @@ protected:
     bool _separator;
 };
 
-class ICE_UTIL_API NextLine { };
+class ICE_UTIL_API NextLine
+{
+};
 extern ICE_UTIL_API NextLine nl;
 
-class ICE_UTIL_API Separator { };
+class ICE_UTIL_API Separator
+{
+};
 extern ICE_UTIL_API Separator sp;
 
 // ----------------------------------------------------------------------
@@ -126,7 +129,9 @@ operator<<(Output& o, const Separator&)
     return o;
 }
 
-class ICE_UTIL_API StartBlock { };
+class ICE_UTIL_API StartBlock
+{
+};
 extern ICE_UTIL_API StartBlock sb;
 
 template<>
@@ -137,7 +142,9 @@ operator<<(Output& o, const StartBlock&)
     return o;
 }
 
-class ICE_UTIL_API EndBlock { };
+class ICE_UTIL_API EndBlock
+{
+};
 extern ICE_UTIL_API EndBlock eb;
 
 template<>
@@ -203,12 +210,13 @@ operator<<(XMLOutput& o, const Separator&)
     return o;
 }
 
+// TODO: Not performance critical, does not need to be inlined.
 class ICE_UTIL_API StartElement
 {
 public:
 
-    StartElement(const std::string& name)
-	: _name(name)
+    StartElement(const std::string& name) :
+	_name(name)
     {
     }
     
@@ -216,7 +224,10 @@ public:
     {
     }
 
-    const std::string& getName() const { return _name; }
+    const std::string& getName() const
+    {
+	return _name;
+    }
 
 private:
 
diff --git a/cpp/include/IceUtil/Shared.h b/cpp/include/IceUtil/Shared.h
index c323cfbfbfc..f54df64ba59 100644
--- a/cpp/include/IceUtil/Shared.h
+++ b/cpp/include/IceUtil/Shared.h
@@ -39,25 +39,32 @@
  * on us. We need to use _exactly_ the address the user gave us,
  * not some alias that contains the same information.
  */
-typedef struct { volatile int counter; } ice_atomic_t;
+struct ice_atomic_t
+{
+    volatile int counter;
+};
 
-/**
+/*
  * ice_atomic_set - set ice_atomic variable
  * @v: pointer of type ice_atomic_t
  * @i: required value
  * 
- * Atomically sets the value of @v to @i.  Note that the guaranteed
+ * Atomically sets the value of @v to @i. Note that the guaranteed
  * useful range of an ice_atomic_t is only 24 bits.
- */ 
-#define ice_atomic_set(v,i) (((v)->counter) = (i))
+ */
+// TODO: Does this really need to be a macro?t
+#define ice_atomic_set(v, i) (((v)->counter) = (i))
 
-/**
+/*
  * ice_atomic_inc - increment ice_atomic variable
  * @v: pointer of type ice_atomic_t
  * 
- * Atomically increments @v by 1.  Note that the guaranteed
- * useful range of an ice_atomic_t is only 24 bits.
- */ 
+ * Atomically increments @v by 1. Note that the guaranteed useful
+ * range of an ice_atomic_t is only 24 bits.
+ *
+ * Inlined because this operation is performance critical.
+ */
+// TODO: Why static?
 static inline void ice_atomic_inc(ice_atomic_t *v)
 {
     __asm__ __volatile__(
@@ -70,11 +77,13 @@ static inline void ice_atomic_inc(ice_atomic_t *v)
  * ice_atomic_dec_and_test - decrement and test
  * @v: pointer of type ice_atomic_t
  * 
- * Atomically decrements @v by 1 and
- * returns true if the result is 0, or false for all other
- * cases.  Note that the guaranteed
- * useful range of an ice_atomic_t is only 24 bits.
- */ 
+ * Atomically decrements @v by 1 and returns true if the result is 0,
+ * or false for all other cases. Note that the guaranteed useful
+ * range of an ice_atomic_t is only 24 bits.
+ *
+ * Inlined because this operation is performance critical.
+ */
+// TODO: Why static?
 static inline int ice_atomic_dec_and_test(ice_atomic_t *v)
 {
     unsigned char c;
@@ -86,10 +95,13 @@ static inline int ice_atomic_dec_and_test(ice_atomic_t *v)
 }
 
 /**
- * ice_atomic_exchange_add - same as InterlockedExchangeAdd. This didn't
- * come from atomic.h (the code was derived from similar code in
- * /usr/include/asm/rwsem.h)
+ * ice_atomic_exchange_add - same as InterlockedExchangeAdd. This
+ * didn't come from atomic.h (the code was derived from similar code
+ * in /usr/include/asm/rwsem.h)
+ *
+ * Inlined because this operation is performance critical.
  */
+// TODO: Why static?
 static inline int ice_atomic_exchange_add(int i, ice_atomic_t* v)
 {
     int tmp = i;
@@ -121,6 +133,11 @@ static inline int ice_atomic_exchange_add(int i, ice_atomic_t* v)
 namespace IceUtil
 {
 
+//
+// TODO: Not all operations in this class are performance critical,
+// thus not all of them should be inlined.
+//
+
 class SimpleShared : public noncopyable
 {
 public:
@@ -193,6 +210,11 @@ private:
     bool _noDelete;
 };
 
+//
+// TODO: Not all operations below are performance critical, thus not
+// all of them should be inlined.
+//
+
 #ifdef ICE_USE_MUTEX_SHARED
 
 inline
diff --git a/cpp/include/IceUtil/Thread.h b/cpp/include/IceUtil/Thread.h
index cb821fc491c..55f1689f73a 100644
--- a/cpp/include/IceUtil/Thread.h
+++ b/cpp/include/IceUtil/Thread.h
@@ -18,12 +18,14 @@ namespace IceUtil
 {
 
 #ifdef WIN32
+// TODO: Should not be inlined, not performance critical.
 struct HandleWrapper : public Shared
 {
     HandleWrapper(HANDLE h) :
 	handle(h)
     {
     }
+
     ~HandleWrapper()
     {
 	if (handle != 0)