Cleanup base/stl_util

base/stl_util-inl.h

Index: base/stl_util-inl.h
diff --git a/base/stl_util-inl.h b/base/stl_util-inl.h
index 2161c59a0330ee241888d22ef07ed2e6d0b398a2..d25b7b07d9b931733155e08aa5c05f2000acc584 100644
--- a/base/stl_util-inl.h
+++ b/base/stl_util-inl.h
@@ -9,44 +9,29 @@
 #define BASE_STL_UTIL_INL_H_
 #pragma once
 
+#include <assert.h>

[brettw, 2011/07/14 15:10:42]

What was the reason for this assert change? Wouldn't cassert be preferred for
C++ code?

[Denis Lagno, 2011/07/14 16:45:26]

1st reason is that it is inconsistent: using <cassert> and <string.h>
simultaneously.  So one of them should be changed .

2nd reason is that in this case it is just confusing: generally <cname> headers
export their stuff in namespace std, however code accesses assert not
std::assert -- that is because assert is actually preprocessor macro not a
function.

 #include <string.h>  // for memcpy
+
 #include <functional>
 #include <set>
 #include <string>
 #include <vector>
-#include <cassert>
 
 // Clear internal memory of an STL object.
 // STL clear()/reserve(0) does not always free internal memory allocated
 // This function uses swap/destructor to ensure the internal memory is freed.
-template<class T> void STLClearObject(T* obj) {
-  T tmp;
-  tmp.swap(*obj);
-  obj->reserve(0);  // this is because sometimes "T tmp" allocates objects with
-                    // memory (arena implementation?).  use reserve()
-                    // to clear() even if it doesn't always work
-}
-
-// Reduce memory usage on behalf of object if it is using more than
-// "bytes" bytes of space.  By default, we clear objects over 1MB.
-template <class T> inline void STLClearIfBig(T* obj, size_t limit = 1<<20) {
-  if (obj->capacity() >= limit) {
-    STLClearObject(obj);
+template<class T> void STLClearObject(T* obj, size_t limit = 1 << 6) {

[brettw, 2011/07/14 15:10:42]

I don't understand what your goal with this change was, and I question why our
implementation should be different than google3/util/gtl/stl_util.h

[Denis Lagno, 2011/07/14 16:45:26]

My main goal was to get rid of STLClearIfBig function -- its name is just
misleading: in fact it clears unconditionally (not only big objects), just for
big objects it tries harder to reclaim occupied memory.

On second thought I decided just to move previous functionality of STLClearIfBig
into STLClearObject -- because they actually have the same semantics (clear
object) with different memory reclaiming behaviour.

But ok, I've just reverted to previous behaviour of STLClearObject.

+  if (obj->capacity() > limit) {
+    T tmp;
+    tmp.swap(*obj);
+    // Sometimes "T tmp" allocates objects with memory (arena implementation?).
+    // Hence using additional reserve(0) even if it doesn't always work.
+    obj->reserve(0);
   } else {
     obj->clear();
   }
 }
 
-// Reserve space for STL object.
-// STL's reserve() will always copy.
-// This function avoid the copy if we already have capacity
-template<class T> void STLReserveIfNeeded(T* obj, int new_size) {
-  if (obj->capacity() < new_size)   // increase capacity
-    obj->reserve(new_size);
-  else if (obj->size() > new_size)  // reduce size
-    obj->resize(new_size);
-}
-
 // STLDeleteContainerPointers()
 //  For a range within a container of pointers, calls delete
 //  (non-array version) on these pointers.
@@ -111,63 +96,30 @@ void STLDeleteContainerPairSecondPointers(ForwardIterator begin,
   }
 }
 
-template<typename T>
-inline void STLAssignToVector(std::vector<T>* vec,
-                              const T* ptr,
-                              size_t n) {
-  vec->resize(n);
-  memcpy(&vec->front(), ptr, n*sizeof(T));
-}
-
-/***** Hack to allow faster assignment to a vector *****/
-
-// This routine speeds up an assignment of 32 bytes to a vector from
-// about 250 cycles per assignment to about 140 cycles.
-//
-// Usage:
-//      STLAssignToVectorChar(&vec, ptr, size);
-//      STLAssignToString(&str, ptr, size);
-
-inline void STLAssignToVectorChar(std::vector<char>* vec,
-                                  const char* ptr,
-                                  size_t n) {
-  STLAssignToVector(vec, ptr, n);
-}
-
-inline void STLAssignToString(std::string* str, const char* ptr, size_t n) {
-  str->resize(n);
-  memcpy(&*str->begin(), ptr, n);
-}
-
 // To treat a possibly-empty vector as an array, use these functions.
-// If you know the array will never be empty, you can use &*v.begin()
-// directly, but that is allowed to dump core if v is empty.  This
-// function is the most efficient code that will work, taking into
-// account how our STL is actually implemented.  THIS IS NON-PORTABLE
-// CODE, so call us instead of repeating the nonportable code
-// everywhere.  If our STL implementation changes, we will need to
-// change this as well.
+// If you know the array will never be empty, you can use &v.front()
+// directly, but that is undefined behaviour if v is empty.
 
 template<typename T>
 inline T* vector_as_array(std::vector<T>* v) {
 # ifdef NDEBUG
-  return &*v->begin();
+  return &v->front();

[brettw, 2011/07/14 15:10:42]

I wouldn't change these, I think the generated code should be the same, and it
makes us diverge unnecessarily from the google3 version.

[Denis Lagno, 2011/07/14 16:45:26]

Done.

 # else
-  return v->empty() ? NULL : &*v->begin();
+  return v->empty() ? NULL : &v->front();
 # endif
 }
 
 template<typename T>
 inline const T* vector_as_array(const std::vector<T>* v) {
 # ifdef NDEBUG
-  return &*v->begin();
+  return &v->front();
 # else
-  return v->empty() ? NULL : &*v->begin();
+  return v->empty() ? NULL : &v->front();
 # endif
 }
 
 // Return a mutable char* pointing to a string's internal buffer,
-// which may not be null-terminated. Writing through this pointer will
+// which may be not null-terminated. Writing through this pointer will
 // modify the string.
 //
 // string_as_array(&str)[i] is valid for 0 <= i < str.size() until the
@@ -179,10 +131,21 @@ inline const T* vector_as_array(const std::vector<T>* v) {
 // proposes this as the method. According to Matt Austern, this should
 // already work on all current implementations.
 inline char* string_as_array(std::string* str) {
-  // DO NOT USE const_cast<char*>(str->data())! See the unittest for why.
+  // DO NOT USE const_cast<char*>(str->data())
   return str->empty() ? NULL : &*str->begin();
 }
 
+template<typename T>
+inline void STLAssignToVector(std::vector<T>* vec, const T* ptr, size_t n) {
+  vec->resize(n);
+  memcpy(vector_as_array(vec), ptr, n*sizeof(T));
+}
+
+inline void STLAssignToString(std::string* str, const char* ptr, size_t n) {
+  str->resize(n);
+  memcpy(string_as_array(str), ptr, n);
+}
+
 // These are methods that test two hash maps/sets for equality.  These exist
 // because the == operator in the STL can return false when the maps/sets
 // contain identical elements.  This is because it compares the internal hash