Cleanup base/stl_util

base/stl_util-inl.h

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // STL utility functions.  Usually, these replace built-in, but slow(!),
 // STL functions with more efficient versions.
 
 #ifndef BASE_STL_UTIL_INL_H_
 #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.
 // NOTE: for these three functions, we could just implement a DeleteObject
 // functor and then call for_each() on the range and functor, but this
 // requires us to pull in all of algorithm.h, which seems expensive.
 // For hash_[multi]set, it is important that this deletes behind the iterator
 // because the hash_set may call the hash function on the iterator when it is
 // advanced, which could result in the hash function trying to deference a
 // stale pointer.
@@ skipping 44 matching lines @@
 //  (non-array version) on the SECOND item in the pairs.
 template <class ForwardIterator>
 void STLDeleteContainerPairSecondPointers(ForwardIterator begin,
                                           ForwardIterator end) {
   while (begin != end) {
     delete begin->second;
     ++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
 // next call to a string method that invalidates iterators.
 //
 // As of 2006-04, there is no standard-blessed way of getting a
 // mutable reference to a string's internal buffer. However, issue 530
 // (http://www.open-std.org/JTC1/SC22/WG21/docs/lwg-active.html#530)
 // 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
 // tables which may be different if the order of insertions and deletions
 // differed.
 
 template <class HashSet>
 inline bool HashSetEquality(const HashSet& set_a, const HashSet& set_b) {
   if (set_a.size() != set_b.size()) return false;
   for (typename HashSet::const_iterator i = set_a.begin();
@@ skipping 247 matching lines @@
 }
 
 // Test to see if a set, map, hash_set or hash_map contains a particular key.
 // Returns true if the key is in the collection.
 template <typename Collection, typename Key>
 bool ContainsKey(const Collection& collection, const Key& key) {
   return collection.find(key) != collection.end();
 }
 
 #endif  // BASE_STL_UTIL_INL_H_