Bug 1087245 part 3 - Avoid memory allocation when initializating Poison IO File Handle list so that LogAlloc doesn't deadlock at initialization. r=nfroyd
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 18 Nov 2014 19:21:26 +0900
changeset 240539 4fd3e61b08b42eea3ccad80f62b2222282daa84a
parent 240538 8e41796f717d4169a4e725afe844e4894b209399
child 240540 ecc30ea0f2c136612eb5d9aa972ad692cbf6414a
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs1087245
milestone36.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 1087245 part 3 - Avoid memory allocation when initializating Poison IO File Handle list so that LogAlloc doesn't deadlock at initialization. r=nfroyd
xpcom/build/PoisonIOInterposerBase.cpp
--- a/xpcom/build/PoisonIOInterposerBase.cpp
+++ b/xpcom/build/PoisonIOInterposerBase.cpp
@@ -1,19 +1,19 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim:set ts=2 sw=2 sts=2 ci et: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/Mutex.h"
 #include "mozilla/Scoped.h"
+#include "mozilla/UniquePtr.h"
 
 #include <algorithm>
-#include <vector>
 
 #include "PoisonIOInterposer.h"
 
 #ifdef MOZ_REPLACE_MALLOC
 #include "replace_malloc_bridge.h"
 #endif
 
 // Auxiliary method to convert file descriptors to ids
@@ -74,70 +74,166 @@ public:
 PRLock* DebugFilesAutoLock::Lock;
 void
 DebugFilesAutoLock::Clear()
 {
   MOZ_ASSERT(Lock != nullptr);
   Lock = nullptr;
 }
 
-// Return a vector used to hold the IDs of the current debug files. On unix
+// The ChunkedList<T> class implements, at the high level, a non-iterable
+// list of instances of T. Its goal is to be somehow minimalist for the
+// use case of storing the debug files handles here, with the property of
+// not requiring a lock to look up whether it contains a specific value.
+// It is also chunked in blocks of chunk_size bytes so that its
+// initialization doesn't require a memory allocation, while keeping the
+// possibility to increase its size as necessary. Note that chunks are
+// never deallocated (except in the destructor).
+// All operations are essentially O(N) but N is not expected to be large
+// enough to matter.
+template <typename T, size_t chunk_size=64>
+class ChunkedList {
+  struct ListChunk {
+    static const size_t kLength = \
+      (chunk_size - sizeof(ListChunk*)) / sizeof(mozilla::Atomic<T>);
+
+    mozilla::Atomic<T> mElements[kLength];
+    mozilla::UniquePtr<ListChunk> mNext;
+
+    ListChunk() : mNext(nullptr) {}
+  };
+
+  ListChunk mList;
+  mozilla::Atomic<size_t> mLength;
+
+public:
+  ChunkedList() : mLength(0) {}
+
+  ~ChunkedList() {
+    // There can be writes happening after this destructor runs, so keep
+    // the list contents and don't reset mLength. But if there are more
+    // elements left than the first chunk can hold, then all hell breaks
+    // loose for any write that would happen after that because any extra
+    // chunk would be deallocated, so just crash in that case.
+    MOZ_RELEASE_ASSERT(mLength <= ListChunk::kLength);
+  }
+
+  // Add an element at the end of the last chunk of the list. Create a new
+  // chunk if there is not enough room.
+  // This is not thread-safe with another thread calling Add or Remove.
+  void Add(T aValue)
+  {
+    ListChunk *list = &mList;
+    size_t position = mLength;
+    for (; position >= ListChunk::kLength; position -= ListChunk::kLength) {
+      if (!list->mNext) {
+        list->mNext.reset(new ListChunk());
+      }
+      list = list->mNext.get();
+    }
+    // Use an order of operations that ensures any racing Contains call
+    // can't be hurt.
+    list->mElements[position] = aValue;
+    mLength++;
+  }
+
+  // Remove an element from the list by replacing it with the last element
+  // of the list, and then shrinking the list.
+  // This is not thread-safe with another thread calling Add or Remove.
+  void Remove(T aValue)
+  {
+    if (!mLength) {
+      return;
+    }
+    ListChunk *list = &mList;
+    size_t last = mLength - 1;
+    do {
+      size_t position = 0;
+      // Look for an element matching the given value.
+      for (; position < ListChunk::kLength; position++) {
+        if (aValue == list->mElements[position]) {
+          ListChunk *last_list = list;
+          // Look for the last element in the list, starting from where we are
+          // instead of starting over.
+          for (; last >= ListChunk::kLength; last -= ListChunk::kLength) {
+            last_list = last_list->mNext.get();
+          }
+          // Use an order of operations that ensures any racing Contains call
+          // can't be hurt.
+          T value = last_list->mElements[last];
+          list->mElements[position] = value;
+          mLength--;
+          return;
+        }
+      }
+      last -= ListChunk::kLength;
+      list = list->mNext.get();
+    } while (list);
+  }
+
+  // Returns whether the list contains the given value. It is meant to be safe
+  // to use without locking, with the tradeoff of being not entirely accurate
+  // if another thread adds or removes an element while this function runs.
+  bool Contains(T aValue)
+  {
+    ListChunk *list = &mList;
+    // Fix the range of the lookup to whatever the list length is when the
+    // function is called.
+    size_t length = mLength;
+    do {
+      size_t list_length = ListChunk::kLength;
+      list_length = std::min(list_length, length);
+      for (size_t position = 0; position < list_length; position++) {
+        if (aValue == list->mElements[position]) {
+          return true;
+        }
+      }
+      length -= ListChunk::kLength;
+      list = list->mNext.get();
+    } while (list);
+
+    return false;
+  }
+};
+
+typedef ChunkedList<intptr_t> FdList;
+
+// Return a list used to hold the IDs of the current debug files. On unix
 // an ID is a file descriptor. On Windows it is a file HANDLE.
-std::vector<intptr_t>*
+FdList&
 getDebugFileIDs()
 {
-  PR_ASSERT_CURRENT_THREAD_OWNS_LOCK(DebugFilesAutoLock::getDebugFileIDsLock());
-  // We have to use new as some write happen during static destructors
-  // so an static std::vector might be destroyed while we still need it.
-  static std::vector<intptr_t>* DebugFileIDs = new std::vector<intptr_t>();
+  static FdList DebugFileIDs;
   return DebugFileIDs;
 }
 
+
 } // anonymous namespace
 
 namespace mozilla {
 
 // Auxiliary Method to test if a file descriptor is registered to be ignored
 // by the poisoning IO interposer
 bool
 IsDebugFile(intptr_t aFileID)
 {
-  DebugFilesAutoLock lockedScope;
-
-  std::vector<intptr_t>& Vec = *getDebugFileIDs();
-  return std::find(Vec.begin(), Vec.end(), aFileID) != Vec.end();
+  return getDebugFileIDs().Contains(aFileID);
 }
 
-// Clean-up for the registered debug files.
-// We should probably make sure all debug files are unregistered instead.
-// But as the poison IO interposer is used for late-write checks we're not
-// disabling it at any point yet. So Really no need for this.
-//
-// void ClearDebugFileRegister() {
-//   PRLock *Lock;
-//   {
-//     DebugFilesAutoLock lockedScope;
-//     delete getDebugFileIDs();
-//     Lock = DebugFilesAutoLock::getDebugFileIDsLock();
-//     DebugFilesAutoLock::Clear();
-//   }
-//   PR_DestroyLock(Lock);
-// }
-
 } // namespace mozilla
 
 extern "C" {
 
 void
 MozillaRegisterDebugHandle(intptr_t aHandle)
 {
   DebugFilesAutoLock lockedScope;
-  std::vector<intptr_t>& Vec = *getDebugFileIDs();
-  MOZ_ASSERT(std::find(Vec.begin(), Vec.end(), aHandle) == Vec.end());
-  Vec.push_back(aHandle);
+  FdList& DebugFileIDs = getDebugFileIDs();
+  MOZ_ASSERT(!DebugFileIDs.Contains(aHandle));
+  DebugFileIDs.Add(aHandle);
 }
 
 void
 MozillaRegisterDebugFD(int aFd)
 {
   MozillaRegisterDebugHandle(FileDescriptorToHandle(aFd));
 }
 
@@ -150,21 +246,19 @@ MozillaRegisterDebugFILE(FILE* aFile)
   }
   MozillaRegisterDebugFD(fd);
 }
 
 void
 MozillaUnRegisterDebugHandle(intptr_t aHandle)
 {
   DebugFilesAutoLock lockedScope;
-  std::vector<intptr_t>& Vec = *getDebugFileIDs();
-  std::vector<intptr_t>::iterator i =
-    std::find(Vec.begin(), Vec.end(), aHandle);
-  MOZ_ASSERT(i != Vec.end());
-  Vec.erase(i);
+  FdList& DebugFileIDs = getDebugFileIDs();
+  MOZ_ASSERT(DebugFileIDs.Contains(aHandle));
+  DebugFileIDs.Remove(aHandle);
 }
 
 void
 MozillaUnRegisterDebugFD(int aFd)
 {
   MozillaUnRegisterDebugHandle(FileDescriptorToHandle(aFd));
 }