Bug 1395509 - Pack MemoryTracker hashtable keys into a single word on 64bit platforms r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 22 May 2019 17:45:03 +0100
changeset 475397 740009023b1b6867a18b8f2f44e0ba84bfa229ec
parent 475396 919893dc7365431d63b09f1603a961e6e5285f20
child 475398 386097a10f84a183ac8b16c4fbc7b391261f1fd2
push id36061
push usercbrindusan@mozilla.com
push dateFri, 24 May 2019 21:49:59 +0000
treeherdermozilla-central@5d3e1ea77693 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1395509
milestone69.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 1395509 - Pack MemoryTracker hashtable keys into a single word on 64bit platforms r=sfink This code is debug-only, but it seems a shame to waste have this key structure take up two words when it will fix into one. This also moves the MemoryUse enum definition to gc/GCEnum.h. Differential Revision: https://phabricator.services.mozilla.com/D32170
js/public/MemoryFunctions.h
js/src/gc/FreeOp.h
js/src/gc/GCEnum.h
js/src/gc/Marking.cpp
js/src/gc/Scheduling.h
js/src/gc/Zone.cpp
--- a/js/public/MemoryFunctions.h
+++ b/js/public/MemoryFunctions.h
@@ -11,16 +11,17 @@
 #include "mozilla/Assertions.h"  // MOZ_ASSERT
 #include "mozilla/Attributes.h"  // MOZ_MUST_USE
 
 #include <stddef.h>  // size_t
 
 #include "jstypes.h"  // JS_PUBLIC_API
 
 struct JSContext;
+class JSObject;
 struct JSRuntime;
 
 struct JSFreeOp {
  protected:
   JSRuntime* runtime_;
 
   explicit JSFreeOp(JSRuntime* rt) : runtime_(rt) {}
 
--- a/js/src/gc/FreeOp.h
+++ b/js/src/gc/FreeOp.h
@@ -4,28 +4,27 @@
  * 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/. */
 
 #ifndef gc_FreeOp_h
 #define gc_FreeOp_h
 
 #include "mozilla/Assertions.h"  // MOZ_ASSERT
 
+#include "gc/GCEnum.h"                // js::MemoryUse
 #include "jit/ExecutableAllocator.h"  // jit::JitPoisonRangeVector
 #include "js/AllocPolicy.h"           // SystemAllocPolicy
 #include "js/MemoryFunctions.h"       // JSFreeOp
 #include "js/Utility.h"               // AutoEnterOOMUnsafeRegion, js_free
 #include "js/Vector.h"                // js::Vector
 
 struct JSRuntime;
 
 namespace js {
 
-enum class MemoryUse : uint8_t;
-
 /*
  * A FreeOp can do one thing: free memory. For convenience, it has delete_
  * convenience methods that also call destructors.
  *
  * FreeOp is passed to finalizers and other sweep-phase hooks so that we do not
  * need to pass a JSContext to those hooks.
  */
 class FreeOp : public JSFreeOp {
--- a/js/src/gc/GCEnum.h
+++ b/js/src/gc/GCEnum.h
@@ -8,16 +8,18 @@
  * GC-internal enum definitions.
  */
 
 #ifndef gc_GCEnum_h
 #define gc_GCEnum_h
 
 #include <stdint.h>
 
+#include "js/MemoryFunctions.h" // JS_FOR_EACH_PUBLIC_MEMORY_USE
+
 namespace js {
 namespace gc {
 
 // Mark colors to pass to markIfUnmarked.
 enum class MarkColor : uint32_t { Black = 0, Gray };
 
 // The phases of an incremental GC.
 #define GCSTATES(D) \
@@ -82,11 +84,26 @@ enum class ZealMode {
 #define ZEAL_MODE(name, value) name = value,
   JS_FOR_EACH_ZEAL_MODE(ZEAL_MODE)
 #undef ZEAL_MODE
       Count,
   Limit = Count - 1
 };
 
 } /* namespace gc */
+
+#define JS_FOR_EACH_INTERNAL_MEMORY_USE(_)      \
+  _(ArrayBufferContents)                        \
+  _(StringContents)
+
+#define JS_FOR_EACH_MEMORY_USE(_)               \
+  JS_FOR_EACH_PUBLIC_MEMORY_USE(_)              \
+  JS_FOR_EACH_INTERNAL_MEMORY_USE(_)
+
+enum class MemoryUse : uint8_t {
+#define DEFINE_MEMORY_USE(Name) Name,
+  JS_FOR_EACH_MEMORY_USE(DEFINE_MEMORY_USE)
+#undef DEFINE_MEMORY_USE
+};
+
 } /* namespace js */
 
 #endif /* gc_GCEnum_h */
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -3099,18 +3099,17 @@ size_t js::TenuringTracer::moveStringToT
   if (!src->isInline() && src->isLinear()) {
     if (src->isUndepended() || !src->hasBase()) {
       void* chars = src->asLinear().nonInlineCharsRaw();
       nursery().removeMallocedBuffer(chars);
     }
   }
 
   if (dst->isFlat() && !dst->isInline()) {
-    AddCellMemory(dst, dst->asFlat().allocSize(),
-                  MemoryUse::StringContents);
+    AddCellMemory(dst, dst->asFlat().allocSize(), MemoryUse::StringContents);
   }
 
   return size;
 }
 
 /*** IsMarked / IsAboutToBeFinalized ****************************************/
 
 template <typename T>
--- a/js/src/gc/Scheduling.h
+++ b/js/src/gc/Scheduling.h
@@ -308,31 +308,16 @@
 #define gc_Scheduling_h
 
 #include "mozilla/Atomics.h"
 #include "mozilla/DebugOnly.h"
 
 #include "js/HashTable.h"
 
 namespace js {
-
-#define JS_FOR_EACH_INTERNAL_MEMORY_USE(_)      \
-  _(ArrayBufferContents)                        \
-  _(StringContents)
-
-#define JS_FOR_EACH_MEMORY_USE(_)               \
-  JS_FOR_EACH_PUBLIC_MEMORY_USE(_)              \
-  JS_FOR_EACH_INTERNAL_MEMORY_USE(_)
-
-enum class MemoryUse : uint8_t {
-#define DEFINE_MEMORY_USE(Name) Name,
-  JS_FOR_EACH_MEMORY_USE(DEFINE_MEMORY_USE)
-#undef DEFINE_MEMORY_USE
-};
-
 namespace gc {
 
 struct Cell;
 
 enum TriggerKind { NoTrigger = 0, IncrementalTrigger, NonIncrementalTrigger };
 
 /*
  * Encapsulates all of the GC tunables. These are effectively constant and
@@ -706,18 +691,28 @@ class MemoryTracker {
                   mozilla::recordreplay::Behavior::DontPreserve>
       bytes_;
 
 #ifdef DEBUG
   void trackMemory(Cell* cell, size_t nbytes, MemoryUse use);
   void untrackMemory(Cell* cell, size_t nbytes, MemoryUse use);
 
   struct Key {
-    Cell* cell;
-    MemoryUse use;
+    Key(Cell* cell, MemoryUse use);
+    Cell* cell() const;
+    MemoryUse use() const;
+   private:
+#ifdef JS_64BIT
+    // Pack this into a single word on 64 bit platforms.
+    uintptr_t cell_ : 56;
+    uintptr_t use_ : 8;
+#else
+    uintptr_t cell_ : 32;
+    uintptr_t use_ : 8;
+#endif
   };
 
   struct Hasher {
     using Lookup = Key;
     static HashNumber hash(const Lookup& l);
     static bool match(const Key& key, const Lookup& l);
     static void rekey(Key& k, const Key& newKey);
   };
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -546,19 +546,19 @@ MemoryTracker::~MemoryTracker() {
 
   if (map.empty()) {
     MOZ_ASSERT(bytes() == 0);
     return;
   }
 
   fprintf(stderr, "Missing calls to JS::RemoveAssociatedMemory:\n");
   for (auto r = map.all(); !r.empty(); r.popFront()) {
-    fprintf(stderr, "  %p 0x%zx %s\n", r.front().key().cell,
+    fprintf(stderr, "  %p 0x%zx %s\n", r.front().key().cell(),
             r.front().value(),
-            MemoryUseName(r.front().key().use));
+            MemoryUseName(r.front().key().use()));
   }
 
   MOZ_CRASH();
 }
 
 void MemoryTracker::trackMemory(Cell* cell, size_t nbytes, MemoryUse use) {
   MOZ_ASSERT(cell->isTenured());
 
@@ -597,31 +597,48 @@ void MemoryTracker::untrackMemory(Cell* 
   map.remove(ptr);
 }
 
 void MemoryTracker::fixupAfterMovingGC() {
   // Update the table after we move GC things. We don't use MovableCellHasher
   // because that would create a difference between debug and release builds.
   for (Map::Enum e(map); !e.empty(); e.popFront()) {
     const Key& key = e.front().key();
-    Cell* cell = key.cell;
+    Cell* cell = key.cell();
     if (cell->isForwarded()) {
       cell = gc::RelocationOverlay::fromCell(cell)->forwardingAddress();
-      e.rekeyFront(Key{cell, key.use});
+      e.rekeyFront(Key{cell, key.use()});
     }
   }
 }
 
+inline MemoryTracker::Key::Key(Cell* cell, MemoryUse use)
+    : cell_(uint64_t(cell)), use_(uint64_t(use))
+{
+#ifdef JS_64BIT
+  static_assert(sizeof(Key) == 8, "MemoryTracker::Key should be packed into 8 bytes");
+#endif
+  MOZ_ASSERT(this->cell() == cell);
+  MOZ_ASSERT(this->use() == use);
+}
+
+inline Cell* MemoryTracker::Key::cell() const {
+  return reinterpret_cast<Cell*>(cell_);
+}
+inline MemoryUse MemoryTracker::Key::use() const {
+  return static_cast<MemoryUse>(use_);
+}
+
 inline HashNumber MemoryTracker::Hasher::hash(const Lookup& l) {
-  return mozilla::HashGeneric(DefaultHasher<Cell*>::hash(l.cell),
-                              DefaultHasher<unsigned>::hash(unsigned(l.use)));
+  return mozilla::HashGeneric(DefaultHasher<Cell*>::hash(l.cell()),
+                              DefaultHasher<unsigned>::hash(unsigned(l.use())));
 }
 
 inline bool MemoryTracker::Hasher::match(const Key& k, const Lookup& l) {
-  return k.cell == l.cell && k.use == l.use;
+  return k.cell() == l.cell() && k.use() == l.use();
 }
 
 inline void MemoryTracker::Hasher::rekey(Key& k, const Key& newKey) {
   k = newKey;
 }
 
 #endif  // DEBUG