Bug 871596 - part 3 - replace memcpys of POD datatypes in Pickle with something smarter; r=bent
authorNathan Froyd <froydnj@mozilla.com>
Wed, 05 Jun 2013 13:01:05 -0400
changeset 135627 e582f705dafaf0b98aff409018145a0289074d71
parent 135626 3e821272288855c7f4e28232a71a1972733843eb
child 135628 a111fc485061e2d44f8a98a54afce72f2244c680
push id24847
push userkwierso@gmail.com
push dateWed, 19 Jun 2013 23:38:15 +0000
treeherdermozilla-central@8ea92aeab783 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs871596
milestone24.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 871596 - part 3 - replace memcpys of POD datatypes in Pickle with something smarter; r=bent
ipc/chromium/src/base/pickle.cc
--- a/ipc/chromium/src/base/pickle.cc
+++ b/ipc/chromium/src/base/pickle.cc
@@ -1,29 +1,104 @@
 // Copyright (c) 2006-2008 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.
 
 #include "base/pickle.h"
 
+#include "mozilla/Endian.h"
+#include "mozilla/TypeTraits.h"
+#include "mozilla/Util.h"
+
 #include <stdlib.h>
 
 #include <limits>
 #include <string>
 
 //------------------------------------------------------------------------------
 
+MOZ_STATIC_ASSERT(MOZ_ALIGNOF(Pickle::memberAlignmentType) >= MOZ_ALIGNOF(uint32_t),
+		  "Insufficient alignment");
+
 // static
 const int Pickle::kPayloadUnit = 64;
 
 // We mark a read only pickle with a special capacity_.
 static const uint32_t kCapacityReadOnly = (uint32_t) -1;
 
 static const char kBytePaddingMarker = char(0xbf);
 
+namespace {
+
+// We want to copy data to our payload as efficiently as possible.
+// memcpy fits the bill for copying, but not all compilers or
+// architectures support inlining memcpy from void*, which has unknown
+// static alignment.  However, we know that all the members of our
+// payload will be aligned on memberAlignmentType boundaries.  We
+// therefore use that knowledge to construct a copier that will copy
+// efficiently (via standard C++ assignment mechanisms) if the datatype
+// needs that alignment or less, and memcpy otherwise.  (The compiler
+// may still inline memcpy, of course.)
+
+template<typename T, size_t size, bool hasSufficientAlignment>
+struct Copier
+{
+  static void Copy(T* dest, void** iter) {
+    memcpy(dest, *iter, sizeof(T));
+  }
+};
+
+// Copying 64-bit quantities happens often enough and can easily be made
+// worthwhile on 32-bit platforms, so handle it specially.  Only do it
+// if 64-bit types aren't sufficiently aligned; the alignment
+// requirements for them vary between 32-bit platforms.
+#ifndef HAVE_64BIT_OS
+template<typename T>
+struct Copier<T, sizeof(uint64_t), false>
+{
+  static void Copy(T* dest, void** iter) {
+#if MOZ_LITTLE_ENDIAN
+    static const int loIndex = 0, hiIndex = 1;
+#else
+    static const int loIndex = 1, hiIndex = 0;
+#endif
+    MOZ_STATIC_ASSERT(MOZ_ALIGNOF(uint32_t*) == MOZ_ALIGNOF(void*),
+		      "Pointers have different alignments");
+    uint32_t* src = *reinterpret_cast<uint32_t**>(iter);
+    uint32_t* uint32dest = reinterpret_cast<uint32_t*>(dest);
+    uint32dest[loIndex] = src[loIndex];
+    uint32dest[hiIndex] = src[hiIndex];
+  }
+};
+#endif
+
+template<typename T, size_t size>
+struct Copier<T, size, true>
+{
+  static void Copy(T* dest, void** iter) {
+    // The reinterpret_cast is only safe if two conditions hold:
+    // (1) If the alignment of T* is the same as void*;
+    // (2) The alignment of the data in *iter is at least as
+    //     big as MOZ_ALIGNOF(T).
+    // Check the first condition, as the second condition is already
+    // known to be true, or we wouldn't be here.
+    MOZ_STATIC_ASSERT(MOZ_ALIGNOF(T*) == MOZ_ALIGNOF(void*),
+		      "Pointers have different alignments");
+    *dest = *(*reinterpret_cast<T**>(iter));
+  }
+};
+
+template<typename T>
+void CopyFromIter(T* dest, void** iter) {
+  MOZ_STATIC_ASSERT(mozilla::IsPod<T>::value, "Copied type must be a POD type");
+  Copier<T, sizeof(T), (MOZ_ALIGNOF(T) <= sizeof(Pickle::memberAlignmentType))>::Copy(dest, iter);
+}
+
+} // anonymous namespace
+
 // Payload is sizeof(Pickle::memberAlignmentType) aligned.
 
 Pickle::Pickle()
     : header_(NULL),
       header_size_(sizeof(Header)),
       capacity_(0),
       variable_buffer_offset_(0) {
   Resize(kPayloadUnit);
@@ -93,67 +168,62 @@ bool Pickle::ReadBool(void** iter, bool*
 bool Pickle::ReadInt16(void** iter, int16_t* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  memcpy(result, *iter, sizeof(*result));
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 bool Pickle::ReadUInt16(void** iter, uint16_t* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  memcpy(result, *iter, sizeof(*result));
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 bool Pickle::ReadInt(void** iter, int* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  // TODO(jar) bug 1129285: Pickle should be cleaned up, and not dependent on
-  // alignment.
-  // Next line is otherwise the same as: memcpy(result, *iter, sizeof(*result));
-  *result = *reinterpret_cast<int*>(*iter);
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 // Always written as a 64-bit value since the size for this type can
 // differ between architectures.
 bool Pickle::ReadLong(void** iter, long* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   int64_t bigResult = 0;
   if (!IteratorHasRoomFor(*iter, sizeof(bigResult)))
     return false;
 
-  // TODO(jar) bug 1129285: Pickle should be cleaned up, and not dependent on
-  // alignment.
-  memcpy(&bigResult, *iter, sizeof(bigResult));
+  CopyFromIter(&bigResult, iter);
   DCHECK(bigResult <= LONG_MAX && bigResult >= LONG_MIN);
   *result = static_cast<long>(bigResult);
 
   UpdateIter(iter, sizeof(bigResult));
   return true;
 }
 
 // Always written as a 64-bit value since the size for this type can
@@ -162,19 +232,17 @@ bool Pickle::ReadULong(void** iter, unsi
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   uint64_t bigResult = 0;
   if (!IteratorHasRoomFor(*iter, sizeof(bigResult)))
     return false;
 
-  // TODO(jar) bug 1129285: Pickle should be cleaned up, and not dependent on
-  // alignment.
-  memcpy(&bigResult, *iter, sizeof(bigResult));
+  CopyFromIter(&bigResult, iter);
   DCHECK(bigResult <= ULONG_MAX);
   *result = static_cast<unsigned long>(bigResult);
 
   UpdateIter(iter, sizeof(bigResult));
   return true;
 }
 
 bool Pickle::ReadLength(void** iter, int* result) const {
@@ -189,124 +257,122 @@ bool Pickle::ReadSize(void** iter, size_
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   uint64_t bigResult = 0;
   if (!IteratorHasRoomFor(*iter, sizeof(bigResult)))
     return false;
 
-  // TODO(jar) bug 1129285: Pickle should be cleaned up, and not dependent on
-  // alignment.
-  memcpy(&bigResult, *iter, sizeof(bigResult));
+  CopyFromIter(&bigResult, iter);
   DCHECK(bigResult <= std::numeric_limits<size_t>::max());
   *result = static_cast<size_t>(bigResult);
 
   UpdateIter(iter, sizeof(bigResult));
   return true;
 }
 
 bool Pickle::ReadInt32(void** iter, int32_t* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  memcpy(result, *iter, sizeof(*result));
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 bool Pickle::ReadUInt32(void** iter, uint32_t* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  memcpy(result, *iter, sizeof(*result));
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 bool Pickle::ReadInt64(void** iter, int64_t* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  memcpy(result, *iter, sizeof(*result));
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 bool Pickle::ReadUInt64(void** iter, uint64_t* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  memcpy(result, *iter, sizeof(*result));
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 bool Pickle::ReadDouble(void** iter, double* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  memcpy(result, *iter, sizeof(*result));
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 // Always written as a 64-bit value since the size for this type can
 // differ between architectures.
 bool Pickle::ReadIntPtr(void** iter, intptr_t* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   int64_t bigResult = 0;
   if (!IteratorHasRoomFor(*iter, sizeof(bigResult)))
     return false;
 
-  memcpy(&bigResult, *iter, sizeof(bigResult));
+  CopyFromIter(&bigResult, iter);
   DCHECK(bigResult <= std::numeric_limits<intptr_t>::max() && bigResult >= std::numeric_limits<intptr_t>::min());
   *result = static_cast<intptr_t>(bigResult);
 
   UpdateIter(iter, sizeof(bigResult));
   return true;
 }
 
 bool Pickle::ReadUnsignedChar(void** iter, unsigned char* result) const {
   DCHECK(iter);
   if (!*iter)
     *iter = const_cast<char*>(payload());
 
   if (!IteratorHasRoomFor(*iter, sizeof(*result)))
     return false;
 
-  memcpy(result, *iter, sizeof(*result));
+  CopyFromIter(result, iter);
 
   UpdateIter(iter, sizeof(*result));
   return true;
 }
 
 bool Pickle::ReadString(void** iter, std::string* result) const {
   DCHECK(iter);
   if (!*iter)