Commit 368614f7 authored by Jon Coppeard's avatar Jon Coppeard
Browse files

Bug 1425450 - Use a per-zone vector of JS holders where possible r=mccr8

Currently the JS holders table is represented as a map which contains pointers to entries in a SegmentedVector.  This patch keeps the single map but use a vector per zone and also has a catch-all vector for where we don't know the zone or the holder can have pointers to more than one zone.

Differential Revision: https://phabricator.services.mozilla.com/D68522

--HG--
extra : moz-landing-system : lando
parent eff4a09d
Loading
Loading
Loading
Loading
+99 −40
Original line number Diff line number Diff line
@@ -474,34 +474,61 @@ static void MozCrashWarningReporter(JSContext*, JSErrorReport*) {
  MOZ_CRASH("Why is someone touching JSAPI without an AutoJSAPI?");
}

JSHolderMap::Entry::Entry() : Entry(nullptr, nullptr, nullptr) {}

JSHolderMap::Entry::Entry(void* aHolder, nsScriptObjectTracer* aTracer,
                          JS::Zone* aZone)
    : mHolder(aHolder),
      mTracer(aTracer)
#ifdef DEBUG
      ,
      mZone(aZone)
#endif
{
}

JSHolderMap::JSHolderMap() : mJSHolderMap(256) {}

template <typename F>
inline void JSHolderMap::ForEach(F&& f) {
  for (auto iter = mJSHolders.Iter(); !iter.Done(); iter.Next()) {
  ForEach(mAnyZoneJSHolders, f, nullptr);
  for (auto i = mPerZoneJSHolders.modIter(); !i.done(); i.next()) {
    EntryVector* holders = i.get().value().get();
    ForEach(*holders, f, i.get().key());
    if (holders->IsEmpty()) {
      i.remove();
    }
  }
}

template <typename F>
inline void JSHolderMap::ForEach(EntryVector& aJSHolders, const F& f,
                                 JS::Zone* aZone) {
  for (auto iter = aJSHolders.Iter(); !iter.Done(); iter.Next()) {
    Entry* entry = &iter.Get();

    // If the entry has been cleared, remove it and shrink the vector.
    if (!entry->mHolder && !RemoveEntry(entry)) {
    if (!entry->mHolder && !RemoveEntry(aJSHolders, entry)) {
      break;  // Removed the last entry.
    }

    f(iter.Get().mHolder, iter.Get().mTracer);
    MOZ_ASSERT(entry->mZone == aZone);
    f(entry->mHolder, entry->mTracer, aZone);
  }
}

bool JSHolderMap::RemoveEntry(Entry* aEntry) {
bool JSHolderMap::RemoveEntry(EntryVector& aJSHolders, Entry* aEntry) {
  MOZ_ASSERT(aEntry);
  MOZ_ASSERT(!aEntry->mHolder);

  // Remove all dead entries from the end of the vector.
  while (!mJSHolders.GetLast().mHolder && &mJSHolders.GetLast() != aEntry) {
    mJSHolders.PopLast();
  while (!aJSHolders.GetLast().mHolder && &aJSHolders.GetLast() != aEntry) {
    aJSHolders.PopLast();
  }

  // Swap the element we want to remove with the last one and update the hash
  // table.
  Entry* lastEntry = &mJSHolders.GetLast();
  Entry* lastEntry = &aJSHolders.GetLast();
  if (aEntry != lastEntry) {
    MOZ_ASSERT(lastEntry->mHolder);
    *aEntry = *lastEntry;
@@ -509,7 +536,7 @@ bool JSHolderMap::RemoveEntry(Entry* aEntry) {
    MOZ_ALWAYS_TRUE(mJSHolderMap.put(aEntry->mHolder, aEntry));
  }

  mJSHolders.PopLast();
  aJSHolders.PopLast();

  // Return whether aEntry is still in the vector.
  return aEntry != lastEntry;
@@ -525,9 +552,9 @@ inline nsScriptObjectTracer* JSHolderMap::Get(void* aHolder) const {
    return nullptr;
  }

  Entry* info = ptr->value();
  MOZ_ASSERT(info->mHolder == aHolder);
  return info->mTracer;
  Entry* entry = ptr->value();
  MOZ_ASSERT(entry->mHolder == aHolder);
  return entry->mTracer;
}

inline nsScriptObjectTracer* JSHolderMap::GetAndRemove(void* aHolder) {
@@ -538,34 +565,59 @@ inline nsScriptObjectTracer* JSHolderMap::GetAndRemove(void* aHolder) {
    return nullptr;
  }

  Entry* info = ptr->value();
  MOZ_ASSERT(info->mHolder == aHolder);
  nsScriptObjectTracer* tracer = info->mTracer;
  Entry* entry = ptr->value();
  MOZ_ASSERT(entry->mHolder == aHolder);
  nsScriptObjectTracer* tracer = entry->mTracer;

  // Clear the entry's contents. It will be removed during iteration.
  info->mHolder = nullptr;
  info->mTracer = nullptr;
  // Clear the entry's contents. It will be removed during the next iteration.
  *entry = Entry();

  mJSHolderMap.remove(ptr);

  return tracer;
}

inline void JSHolderMap::Put(void* aHolder, nsScriptObjectTracer* aTracer) {
inline void JSHolderMap::Put(void* aHolder, nsScriptObjectTracer* aTracer,
                             JS::Zone* aZone) {
  MOZ_ASSERT(aHolder);
  MOZ_ASSERT(aTracer);

  // Don't associate multi-zone holders with a zone, even if one is supplied.
  if (aTracer->IsMultiZoneJSHolder()) {
    aZone = nullptr;
  }

  auto ptr = mJSHolderMap.lookupForAdd(aHolder);
  if (ptr) {
    Entry* info = ptr->value();
    MOZ_ASSERT(info->mHolder == aHolder);
    MOZ_ASSERT(info->mTracer == aTracer,
    Entry* entry = ptr->value();
#ifdef DEBUG
    MOZ_ASSERT(entry->mHolder == aHolder);
    MOZ_ASSERT(entry->mTracer == aTracer,
               "Don't call HoldJSObjects in superclass ctors");
    info->mTracer = aTracer;
    if (aZone) {
      if (entry->mZone) {
        MOZ_ASSERT(entry->mZone == aZone);
      } else {
        entry->mZone = aZone;
      }
    }
#endif
    entry->mTracer = aTracer;
    return;
  }

  mJSHolders.InfallibleAppend(Entry{aHolder, aTracer});
  MOZ_ALWAYS_TRUE(mJSHolderMap.add(ptr, aHolder, &mJSHolders.GetLast()));
  EntryVector* vector = &mAnyZoneJSHolders;
  if (aZone) {
    auto ptr = mPerZoneJSHolders.lookupForAdd(aZone);
    if (!ptr) {
      MOZ_ALWAYS_TRUE(
          mPerZoneJSHolders.add(ptr, aZone, MakeUnique<EntryVector>()));
    }
    vector = ptr->value().get();
  }

  vector->InfallibleAppend(Entry{aHolder, aTracer, aZone});
  MOZ_ALWAYS_TRUE(mJSHolderMap.add(ptr, aHolder, &vector->GetLast()));
}

size_t JSHolderMap::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const {
@@ -573,8 +625,12 @@ size_t JSHolderMap::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const {

  // We're deliberately not measuring anything hanging off the entries in
  // mJSHolders.
  n += mJSHolders.SizeOfExcludingThis(aMallocSizeOf);
  n += mJSHolderMap.shallowSizeOfExcludingThis(aMallocSizeOf);
  n += mAnyZoneJSHolders.SizeOfExcludingThis(aMallocSizeOf);
  n += mPerZoneJSHolders.shallowSizeOfExcludingThis(aMallocSizeOf);
  for (auto i = mPerZoneJSHolders.iter(); !i.done(); i.next()) {
    n += i.get().value()->SizeOfExcludingThis(aMallocSizeOf);
  }

  return n;
}
@@ -694,9 +750,8 @@ size_t CycleCollectedJSRuntime::SizeOfExcludingThis(
}

void CycleCollectedJSRuntime::UnmarkSkippableJSHolders() {
  mJSHolders.ForEach([](void* holder, nsScriptObjectTracer* tracer) {
    tracer->CanSkip(holder, true);
  });
  mJSHolders.ForEach([](void* holder, nsScriptObjectTracer* tracer,
                        JS::Zone* zone) { tracer->CanSkip(holder, true); });
}

void CycleCollectedJSRuntime::DescribeGCThing(
@@ -880,7 +935,8 @@ void CycleCollectedJSRuntime::TraverseNativeRoots(
  // would hurt to do this after the JS holders.
  TraverseAdditionalNativeRoots(aCb);

  mJSHolders.ForEach([&aCb](void* holder, nsScriptObjectTracer* tracer) {
  mJSHolders.ForEach([&aCb](void* holder, nsScriptObjectTracer* tracer,
                            JS::Zone* zone) {
    bool noteRoot = false;
    if (MOZ_UNLIKELY(aCb.WantAllTraces())) {
      noteRoot = true;
@@ -1106,9 +1162,10 @@ void mozilla::TraceScriptHolder(nsISupports* aHolder, JSTracer* aTracer) {
// JS::Zone.
struct CheckZoneTracer : public TraceCallbacks {
  const char* mClassName;
  mutable JS::Zone* mZone = nullptr;
  mutable JS::Zone* mZone;

  explicit CheckZoneTracer(const char* aClassName) : mClassName(aClassName) {}
  explicit CheckZoneTracer(const char* aClassName, JS::Zone* aZone = nullptr)
      : mClassName(aClassName), mZone(aZone) {}

  void checkZone(JS::Zone* aZone, const char* aName) const {
    if (!mZone) {
@@ -1202,8 +1259,9 @@ struct CheckZoneTracer : public TraceCallbacks {
};

static inline void CheckHolderIsSingleZone(
    void* aHolder, nsCycleCollectionParticipant* aParticipant) {
  CheckZoneTracer tracer(aParticipant->ClassName());
    void* aHolder, nsCycleCollectionParticipant* aParticipant,
    JS::Zone* aZone) {
  CheckZoneTracer tracer(aParticipant->ClassName(), aZone);
  aParticipant->Trace(aHolder, tracer, nullptr);
}

@@ -1214,10 +1272,11 @@ void CycleCollectedJSRuntime::TraceNativeGrayRoots(JSTracer* aTracer) {
  // would hurt to do this after the JS holders.
  TraceAdditionalNativeGrayRoots(aTracer);

  mJSHolders.ForEach([aTracer](void* holder, nsScriptObjectTracer* tracer) {
  mJSHolders.ForEach(
      [aTracer](void* holder, nsScriptObjectTracer* tracer, JS::Zone* zone) {
#ifdef DEBUG
        if (!tracer->IsMultiZoneJSHolder()) {
      CheckHolderIsSingleZone(holder, tracer);
          CheckHolderIsSingleZone(holder, tracer, zone);
        }
#endif
        tracer->Trace(holder, JsGcTracer(), aTracer);
@@ -1227,7 +1286,7 @@ void CycleCollectedJSRuntime::TraceNativeGrayRoots(JSTracer* aTracer) {
void CycleCollectedJSRuntime::AddJSHolder(void* aHolder,
                                          nsScriptObjectTracer* aTracer,
                                          JS::Zone* aZone) {
  mJSHolders.Put(aHolder, aTracer);
  mJSHolders.Put(aHolder, aTracer, aZone);
}

struct ClearJSHolder : public TraceCallbacks {
+30 −6
Original line number Diff line number Diff line
@@ -86,7 +86,7 @@ class JSZoneParticipant : public nsCycleCollectionParticipant {

class IncrementalFinalizeRunnable;

// A map from JS holders to tracer objects, where the values are stored in a
// A map from JS holders to tracer objects, where the values are stored in
// SegmentedVector to speed up iteration.
class JSHolderMap {
 public:
@@ -98,7 +98,7 @@ class JSHolderMap {
  bool Has(void* aHolder) const;
  nsScriptObjectTracer* Get(void* aHolder) const;
  nsScriptObjectTracer* GetAndRemove(void* aHolder);
  void Put(void* aHolder, nsScriptObjectTracer* aTracer);
  void Put(void* aHolder, nsScriptObjectTracer* aTracer, JS::Zone* aZone);

  size_t SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const;

@@ -106,17 +106,41 @@ class JSHolderMap {
  struct Entry {
    void* mHolder;
    nsScriptObjectTracer* mTracer;
  };
#ifdef DEBUG
    JS::Zone* mZone;
#endif

  using EntryVector = SegmentedVector<Entry, 1024, InfallibleAllocPolicy>;
    Entry();
    Entry(void* aHolder, nsScriptObjectTracer* aTracer, JS::Zone* aZone);
  };

  using EntryMap = mozilla::HashMap<void*, Entry*, DefaultHasher<void*>,
                                    InfallibleAllocPolicy>;

  bool RemoveEntry(Entry* aEntry);
  using EntryVector = SegmentedVector<Entry, 256, InfallibleAllocPolicy>;

  using EntryVectorMap =
      mozilla::HashMap<JS::Zone*, UniquePtr<EntryVector>,
                       DefaultHasher<JS::Zone*>, InfallibleAllocPolicy>;

  EntryVector mJSHolders;
  template <typename F>
  void ForEach(EntryVector& aJSHolders, const F& f, JS::Zone* aZone);

  bool RemoveEntry(EntryVector& aJSHolders, Entry* aEntry);

  // A map from a holder pointer to a pointer to an entry in a vector.
  EntryMap mJSHolderMap;

  // A vector of holders not associated with a particular zone or that can
  // contain pointers to GC things in more than one zone.
  EntryVector mAnyZoneJSHolders;

  // A map from a zone to a vector of holders that only contain pointers to GC
  // things in that zone.
  //
  // Currently this will only contain wrapper cache wrappers since these are the
  // only holders to pass a zone parameter through to AddJSHolder.
  EntryVectorMap mPerZoneJSHolders;
};

class CycleCollectedJSRuntime {