Bug 1297981 - Delete BufferList::FlattenBytes and Pickle::FlattenBytes. r=billm
authorKan-Ru Chen <kanru@kanru.info>
Thu, 25 Aug 2016 17:15:38 +0800
changeset 355021 241754bbd8712b5a3e31a5c27a5b3278d50223e9
parent 355020 2615d4df8768f04c219491437f54507f2bc87899
child 355022 12fa18765018727f312b291142dcab4b8f1f0eec
child 355076 1c39b57bd2044e48a00f5a116ce2566aec1d6c73
push id1324
push usermtabara@mozilla.com
push dateMon, 16 Jan 2017 13:07:44 +0000
treeherdermozilla-release@a01c49833940 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1297981
milestone51.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 1297981 - Delete BufferList::FlattenBytes and Pickle::FlattenBytes. r=billm MozReview-Commit-ID: G3a4DN4Lovi
ipc/chromium/src/base/pickle.cc
ipc/chromium/src/base/pickle.h
mfbt/BufferList.h
mfbt/tests/TestBufferList.cpp
--- a/ipc/chromium/src/base/pickle.cc
+++ b/ipc/chromium/src/base/pickle.cc
@@ -391,41 +391,16 @@ bool Pickle::ReadWString(PickleIterator*
   if (!ReadBytesInto(iter, chars.get(), len * sizeof(wchar_t))) {
     return false;
   }
   result->assign(chars.get(), len);
 
   return true;
 }
 
-bool Pickle::FlattenBytes(PickleIterator* iter, const char** data, uint32_t length,
-                          uint32_t alignment) {
-  DCHECK(iter);
-  DCHECK(data);
-  DCHECK(alignment == 4 || alignment == 8);
-  DCHECK(intptr_t(header_) % alignment == 0);
-
-  if (AlignInt(length) < length) {
-    return false;
-  }
-
-  uint32_t padding_len = intptr_t(iter->iter_.Data()) % alignment;
-  if (!iter->iter_.AdvanceAcrossSegments(buffers_, padding_len)) {
-    return false;
-  }
-
-  if (!buffers_.FlattenBytes(iter->iter_, data, length)) {
-    return false;
-  }
-
-  header_ = reinterpret_cast<Header*>(buffers_.Start());
-
-  return iter->iter_.AdvanceAcrossSegments(buffers_, AlignInt(length) - length);
-}
-
 bool Pickle::ExtractBuffers(PickleIterator* iter, size_t length, BufferList* buffers,
                             uint32_t alignment) const
 {
   DCHECK(iter);
   DCHECK(buffers);
   DCHECK(alignment == 4 || alignment == 8);
   DCHECK(intptr_t(header_) % alignment == 0);
 
--- a/ipc/chromium/src/base/pickle.h
+++ b/ipc/chromium/src/base/pickle.h
@@ -106,18 +106,16 @@ class Pickle {
   MOZ_MUST_USE bool ReadInt64(PickleIterator* iter, int64_t* result) const;
   MOZ_MUST_USE bool ReadUInt64(PickleIterator* iter, uint64_t* result) const;
   MOZ_MUST_USE bool ReadDouble(PickleIterator* iter, double* result) const;
   MOZ_MUST_USE bool ReadIntPtr(PickleIterator* iter, intptr_t* result) const;
   MOZ_MUST_USE bool ReadUnsignedChar(PickleIterator* iter, unsigned char* result) const;
   MOZ_MUST_USE bool ReadString(PickleIterator* iter, std::string* result) const;
   MOZ_MUST_USE bool ReadWString(PickleIterator* iter, std::wstring* result) const;
   MOZ_MUST_USE bool ReadBytesInto(PickleIterator* iter, void* data, uint32_t length) const;
-  MOZ_MUST_USE bool FlattenBytes(PickleIterator* iter, const char** data, uint32_t length,
-                                 uint32_t alignment = sizeof(memberAlignmentType));
   MOZ_MUST_USE bool ExtractBuffers(PickleIterator* iter, size_t length, BufferList* buffers,
                                    uint32_t alignment = sizeof(memberAlignmentType)) const;
 
   // Safer version of ReadInt() checks for the result not being negative.
   // Use it for reading the object sizes.
   MOZ_MUST_USE bool ReadLength(PickleIterator* iter, int* result) const;
 
   MOZ_MUST_USE bool ReadSentinel(PickleIterator* iter, uint32_t sentinel) const
--- a/mfbt/BufferList.h
+++ b/mfbt/BufferList.h
@@ -56,20 +56,16 @@ class BufferList : private AllocPolicy
   friend class BufferList;
 
  public:
   // For the convenience of callers, all segments are required to be a multiple
   // of 8 bytes in capacity. Also, every buffer except the last one is required
   // to be full (i.e., size == capacity). Therefore, a byte at offset N within
   // the BufferList and stored in memory at an address A will satisfy
   // (N % Align == A % Align) if Align == 2, 4, or 8.
-  //
-  // NB: FlattenBytes can create non-full segments in the middle of the
-  // list. However, it ensures that these buffers are 8-byte aligned, so the
-  // offset invariant is not violated.
   static const size_t kSegmentAlignment = 8;
 
   // Allocate a BufferList. The BufferList will free all its buffers when it is
   // destroyed. An initial buffer of size aInitialSize and capacity
   // aInitialCapacity is allocated automatically. This data will be contiguous
   // an can be accessed via |Start()|. Subsequent buffers will be allocated with
   // capacity aStandardCapacity.
   BufferList(size_t aInitialSize,
@@ -239,24 +235,16 @@ class BufferList : private AllocPolicy
   // bytes may be split across multiple buffers. Size() is increased by aSize.
   inline bool WriteBytes(const char* aData, size_t aSize);
 
   // Copies possibly non-contiguous byte range starting at aIter into
   // aData. aIter is advanced by aSize bytes. Returns false if it runs out of
   // data before aSize.
   inline bool ReadBytes(IterImpl& aIter, char* aData, size_t aSize) const;
 
-  // FlattenBytes reconfigures the BufferList so that data in the range
-  // [aIter, aIter + aSize) is stored contiguously. A pointer to this data is
-  // returned in aOutData. Returns false if not enough data is available. All
-  // other iterators are invalidated by this method.
-  //
-  // This method requires aIter and aSize to be 8-byte aligned.
-  inline bool FlattenBytes(IterImpl& aIter, const char** aOutData, size_t aSize);
-
   // Return a new BufferList that shares storage with this BufferList. The new
   // BufferList is read-only. It allows iteration over aSize bytes starting at
   // aIter. Borrow can fail, in which case *aSuccess will be false upon
   // return. The borrowed BufferList can use a different AllocPolicy than the
   // original one. However, it is not responsible for freeing buffers, so the
   // AllocPolicy is only used for the buffer vector.
   template<typename BorrowingAllocPolicy>
   BufferList<BorrowingAllocPolicy> Borrow(IterImpl& aIter, size_t aSize, bool* aSuccess,
@@ -367,74 +355,16 @@ BufferList<AllocPolicy>::ReadBytes(IterI
     remaining -= toCopy;
 
     aIter.Advance(*this, toCopy);
   }
 
   return true;
 }
 
-template<typename AllocPolicy>
-bool
-BufferList<AllocPolicy>::FlattenBytes(IterImpl& aIter, const char** aOutData, size_t aSize)
-{
-  MOZ_RELEASE_ASSERT(aSize);
-  MOZ_RELEASE_ASSERT(mOwning);
-
-  if (aIter.HasRoomFor(aSize)) {
-    // If the data is already contiguous, just return a pointer.
-    *aOutData = aIter.Data();
-    aIter.Advance(*this, aSize);
-    return true;
-  }
-
-  // This buffer will become the new contiguous segment.
-  char* buffer = this->template pod_malloc<char>(Size());
-  if (!buffer) {
-    return false;
-  }
-
-  size_t copied = 0;
-  size_t offset;
-  bool found = false;
-  for (size_t i = 0; i < mSegments.length(); i++) {
-    Segment& segment = mSegments[i];
-    memcpy(buffer + copied, segment.Start(), segment.mSize);
-
-    if (i == aIter.mSegment) {
-      offset = copied + (aIter.mData - segment.Start());
-
-      // Do we have aSize bytes after aIter?
-      if (Size() - offset >= aSize) {
-        found = true;
-        *aOutData = buffer + offset;
-
-        aIter.mSegment = 0;
-        aIter.mData = buffer + offset + aSize;
-        aIter.mDataEnd = buffer + Size();
-      }
-    }
-
-    this->free_(segment.mData);
-
-    copied += segment.mSize;
-  }
-
-  mSegments.clear();
-  mSegments.infallibleAppend(Segment(buffer, Size(), Size()));
-
-  if (!found) {
-    aIter.mSegment = 0;
-    aIter.mData = Start();
-    aIter.mDataEnd = Start() + Size();
-  }
-
-  return found;
-}
-
 template<typename AllocPolicy> template<typename BorrowingAllocPolicy>
 BufferList<BorrowingAllocPolicy>
 BufferList<AllocPolicy>::Borrow(IterImpl& aIter, size_t aSize, bool* aSuccess,
                                 BorrowingAllocPolicy aAP) const
 {
   BufferList<BorrowingAllocPolicy> result(aAP);
 
   size_t size = aSize;
--- a/mfbt/tests/TestBufferList.cpp
+++ b/mfbt/tests/TestBufferList.cpp
@@ -162,39 +162,16 @@ int main(void)
 
   iter = bl.Iter();
   MOZ_RELEASE_ASSERT(iter.AdvanceAcrossSegments(bl, kTotalSize - kLastSegmentSize - kStandardCapacity));
   MOZ_RELEASE_ASSERT(iter.RemainingInSegment() == kStandardCapacity);
   iter.Advance(bl, kStandardCapacity);
   MOZ_RELEASE_ASSERT(iter.RemainingInSegment() == kLastSegmentSize);
   MOZ_RELEASE_ASSERT(unsigned(*iter.Data()) == (kTotalSize - kLastSegmentSize - kInitialSize - kSmallWrite) % 37);
 
-  // Flattening.
-
-  const size_t kFlattenSize = 1000;
-
-  const char* flat;
-  iter = bl.Iter();
-  MOZ_RELEASE_ASSERT(iter.AdvanceAcrossSegments(bl, kInitialSize));
-  MOZ_RELEASE_ASSERT(bl.FlattenBytes(iter, &flat, kFlattenSize));
-  MOZ_RELEASE_ASSERT(flat[0] == 0x0a);
-  MOZ_RELEASE_ASSERT(flat[kSmallWrite / 2] == 0x0a);
-  for (size_t i = kSmallWrite; i < kFlattenSize; i++) {
-    MOZ_RELEASE_ASSERT(unsigned(flat[i]) == (i - kSmallWrite) % 37);
-  }
-  MOZ_RELEASE_ASSERT(unsigned(*iter.Data()) == (kFlattenSize - kSmallWrite) % 37);
-
-  const size_t kSecondFlattenSize = 40;
-
-  MOZ_RELEASE_ASSERT(bl.FlattenBytes(iter, &flat, kSecondFlattenSize));
-  for (size_t i = 0; i < kSecondFlattenSize; i++) {
-    MOZ_RELEASE_ASSERT(unsigned(flat[i]) == (i + kFlattenSize - kInitialSize) % 37);
-  }
-  MOZ_RELEASE_ASSERT(iter.Done());
-
   // Clear.
 
   bl.Clear();
   MOZ_RELEASE_ASSERT(bl.Size() == 0);
   MOZ_RELEASE_ASSERT(bl.Iter().Done());
 
   // Move assignment.