Bug 1640977 - Rename some ArenaLists members for clarity r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 26 May 2020 17:31:11 +0000
changeset 532327 2348767fd355b47baf252d0e42187c15995d453b
parent 532326 9734a05d25053a0c6510606980c1aeb3cdb3aadc
child 532328 ff13d34e8f7749bb0afc8e5a96c4df35fc892051
push id37454
push userccoroiu@mozilla.com
push dateWed, 27 May 2020 16:14:31 +0000
treeherdermozilla-central@a1dd9afbfdf5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1640977
milestone78.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 1640977 - Rename some ArenaLists members for clarity r=sfink The arenaLists() method returns an ArenaList, confusing when we also have an ArenaLists class, and arenaListsToSweep() returns an Arena. This gives them clearer names. Differential Revision: https://phabricator.services.mozilla.com/D76876
js/src/gc/Allocator.cpp
js/src/gc/ArenaList-inl.h
js/src/gc/ArenaList.h
js/src/gc/GC.cpp
--- a/js/src/gc/Allocator.cpp
+++ b/js/src/gc/Allocator.cpp
@@ -550,17 +550,17 @@ TenuredCell* ArenaLists::refillFreeListA
 
   mozilla::Maybe<AutoLockGCBgAlloc> maybeLock;
 
   // See if we can proceed without taking the GC lock.
   if (concurrentUse(thingKind) != ConcurrentUse::None) {
     maybeLock.emplace(rt);
   }
 
-  ArenaList& al = arenaLists(thingKind);
+  ArenaList& al = arenaList(thingKind);
   Arena* arena = al.takeNextArena();
   if (arena) {
     // Empty arenas should be immediately freed.
     MOZ_ASSERT(!arena->isEmpty());
 
     return freeLists.setArenaAndAllocate(arena, thingKind);
   }
 
--- a/js/src/gc/ArenaList-inl.h
+++ b/js/src/gc/ArenaList-inl.h
@@ -235,58 +235,58 @@ JSRuntime* js::gc::ArenaLists::runtime()
   return zone_->runtimeFromMainThread();
 }
 
 JSRuntime* js::gc::ArenaLists::runtimeFromAnyThread() {
   return zone_->runtimeFromAnyThread();
 }
 
 js::gc::Arena* js::gc::ArenaLists::getFirstArena(AllocKind thingKind) const {
-  return arenaLists(thingKind).head();
+  return arenaList(thingKind).head();
 }
 
 js::gc::Arena* js::gc::ArenaLists::getFirstArenaToSweep(
     AllocKind thingKind) const {
-  return arenaListsToSweep(thingKind);
+  return arenasToSweep(thingKind);
 }
 
 js::gc::Arena* js::gc::ArenaLists::getFirstSweptArena(
     AllocKind thingKind) const {
   if (thingKind != incrementalSweptArenaKind.ref()) {
     return nullptr;
   }
   return incrementalSweptArenas.ref().head();
 }
 
 js::gc::Arena* js::gc::ArenaLists::getArenaAfterCursor(
     AllocKind thingKind) const {
-  return arenaLists(thingKind).arenaAfterCursor();
+  return arenaList(thingKind).arenaAfterCursor();
 }
 
 bool js::gc::ArenaLists::arenaListsAreEmpty() const {
   for (auto i : AllAllocKinds()) {
     /*
      * The arena cannot be empty if the background finalization is not yet
      * done.
      */
     if (concurrentUse(i) == ConcurrentUse::BackgroundFinalize) {
       return false;
     }
-    if (!arenaLists(i).isEmpty()) {
+    if (!arenaList(i).isEmpty()) {
       return false;
     }
   }
   return true;
 }
 
 void js::gc::ArenaLists::unmarkAll() {
   for (auto i : AllAllocKinds()) {
     /* The background finalization must have stopped at this point. */
     MOZ_ASSERT(concurrentUse(i) == ConcurrentUse::None);
-    for (Arena* arena = arenaLists(i).head(); arena; arena = arena->next) {
+    for (Arena* arena = arenaList(i).head(); arena; arena = arena->next) {
       arena->unmarkAll();
     }
   }
 }
 
 bool js::gc::ArenaLists::doneBackgroundFinalize(AllocKind kind) const {
   return concurrentUse(kind) != ConcurrentUse::BackgroundFinalize;
 }
--- a/js/src/gc/ArenaList.h
+++ b/js/src/gc/ArenaList.h
@@ -245,20 +245,18 @@ class FreeLists {
 
 class ArenaLists {
   JS::Zone* zone_;
 
   ZoneData<FreeLists> freeLists_;
 
   ArenaListData<AllAllocKindArray<ArenaList>> arenaLists_;
 
-  ArenaList& arenaLists(AllocKind i) { return arenaLists_.ref()[i]; }
-  const ArenaList& arenaLists(AllocKind i) const {
-    return arenaLists_.ref()[i];
-  }
+  ArenaList& arenaList(AllocKind i) { return arenaLists_.ref()[i]; }
+  const ArenaList& arenaList(AllocKind i) const { return arenaLists_.ref()[i]; }
 
   enum class ConcurrentUse : uint32_t {
     None,
     BackgroundFinalize,
     ParallelAlloc
   };
 
   using ConcurrentUseState =
@@ -270,21 +268,19 @@ class ArenaLists {
   ConcurrentUseState& concurrentUse(AllocKind i) {
     return concurrentUseState_.ref()[i];
   }
   ConcurrentUse concurrentUse(AllocKind i) const {
     return concurrentUseState_.ref()[i];
   }
 
   /* For each arena kind, a list of arenas remaining to be swept. */
-  MainThreadOrGCTaskData<AllAllocKindArray<Arena*>> arenaListsToSweep_;
-  Arena*& arenaListsToSweep(AllocKind i) { return arenaListsToSweep_.ref()[i]; }
-  Arena* arenaListsToSweep(AllocKind i) const {
-    return arenaListsToSweep_.ref()[i];
-  }
+  MainThreadOrGCTaskData<AllAllocKindArray<Arena*>> arenasToSweep_;
+  Arena*& arenasToSweep(AllocKind i) { return arenasToSweep_.ref()[i]; }
+  Arena* arenasToSweep(AllocKind i) const { return arenasToSweep_.ref()[i]; }
 
   /* During incremental sweeping, a list of the arenas already swept. */
   ZoneOrGCTaskData<AllocKind> incrementalSweptArenaKind;
   ZoneOrGCTaskData<ArenaList> incrementalSweptArenas;
 
   // Arena lists which have yet to be swept, but need additional foreground
   // processing before they are swept.
   ZoneData<Arena*> gcShapeArenasToUpdate;
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -2040,40 +2040,40 @@ bool ArenaLists::relocateArenas(Arena*& 
   AllocKinds allocKindsToRelocate = CompactingAllocKinds();
 
   // Clear all the free lists.
   clearFreeLists();
 
   if (ShouldRelocateAllArenas(reason)) {
     zone_->prepareForCompacting();
     for (auto kind : allocKindsToRelocate) {
-      ArenaList& al = arenaLists(kind);
+      ArenaList& al = arenaList(kind);
       Arena* allArenas = al.head();
       al.clear();
       relocatedListOut =
           al.relocateArenas(allArenas, relocatedListOut, sliceBudget, stats);
     }
   } else {
     size_t arenaCount = 0;
     size_t relocCount = 0;
     AllAllocKindArray<Arena**> toRelocate;
 
     for (auto kind : allocKindsToRelocate) {
       toRelocate[kind] =
-          arenaLists(kind).pickArenasToRelocate(arenaCount, relocCount);
+          arenaList(kind).pickArenasToRelocate(arenaCount, relocCount);
     }
 
     if (!ShouldRelocateZone(arenaCount, relocCount, reason)) {
       return false;
     }
 
     zone_->prepareForCompacting();
     for (auto kind : allocKindsToRelocate) {
       if (toRelocate[kind]) {
-        ArenaList& al = arenaLists(kind);
+        ArenaList& al = arenaList(kind);
         Arena* arenas = al.removeRemainingArenas(toRelocate[kind]);
         relocatedListOut =
             al.relocateArenas(arenas, relocatedListOut, sliceBudget, stats);
       }
     }
   }
 
   return true;
@@ -2093,17 +2093,17 @@ bool GCRuntime::relocateArenas(Zone* zon
                                    stats())) {
     return false;
   }
 
 #ifdef DEBUG
   // Check that we did as much compaction as we should have. There
   // should always be less than one arena's worth of free cells.
   for (auto kind : CompactingAllocKinds()) {
-    ArenaList& al = zone->arenas.arenaLists(kind);
+    ArenaList& al = zone->arenas.arenaList(kind);
     size_t freeCells = 0;
     for (Arena* arena = al.arenaAfterCursor(); arena; arena = arena->next) {
       freeCells += arena->countFreeCells();
     }
     MOZ_ASSERT(freeCells < Arena::thingsPerArena(kind));
   }
 #endif
 
@@ -2668,27 +2668,27 @@ FreeLists::FreeLists() {
     freeLists_[i] = &emptySentinel;
   }
 }
 
 ArenaLists::ArenaLists(Zone* zone)
     : zone_(zone),
       freeLists_(zone),
       arenaLists_(zone),
-      arenaListsToSweep_(),
+      arenasToSweep_(),
       incrementalSweptArenaKind(zone, AllocKind::LIMIT),
       incrementalSweptArenas(zone),
       gcShapeArenasToUpdate(zone, nullptr),
       gcAccessorShapeArenasToUpdate(zone, nullptr),
       gcScriptArenasToUpdate(zone, nullptr),
       gcObjectGroupArenasToUpdate(zone, nullptr),
       savedEmptyArenas(zone, nullptr) {
   for (auto i : AllAllocKinds()) {
     concurrentUse(i) = ConcurrentUse::None;
-    arenaListsToSweep(i) = nullptr;
+    arenasToSweep(i) = nullptr;
   }
 }
 
 void ReleaseArenaList(JSRuntime* rt, Arena* arena, const AutoLockGC& lock) {
   Arena* next;
   for (; arena; arena = next) {
     next = arena->next;
     rt->gc.releaseArena(arena, lock);
@@ -2699,17 +2699,17 @@ ArenaLists::~ArenaLists() {
   AutoLockGC lock(runtime());
 
   for (auto i : AllAllocKinds()) {
     /*
      * We can only call this during the shutdown after the last GC when
      * the background finalization is disabled.
      */
     MOZ_ASSERT(concurrentUse(i) == ConcurrentUse::None);
-    ReleaseArenaList(runtime(), arenaLists(i).head(), lock);
+    ReleaseArenaList(runtime(), arenaList(i).head(), lock);
   }
   ReleaseArenaList(runtime(), incrementalSweptArenas.ref().head(), lock);
 
   ReleaseArenaList(runtime(), savedEmptyArenas, lock);
 }
 
 void ArenaLists::queueForForegroundSweep(JSFreeOp* fop,
                                          const FinalizePhase& phase) {
@@ -2717,42 +2717,42 @@ void ArenaLists::queueForForegroundSweep
   for (auto kind : phase.kinds) {
     queueForForegroundSweep(kind);
   }
 }
 
 void ArenaLists::queueForForegroundSweep(AllocKind thingKind) {
   MOZ_ASSERT(!IsBackgroundFinalized(thingKind));
   MOZ_ASSERT(concurrentUse(thingKind) == ConcurrentUse::None);
-  MOZ_ASSERT(!arenaListsToSweep(thingKind));
-
-  arenaListsToSweep(thingKind) = arenaLists(thingKind).head();
-  arenaLists(thingKind).clear();
+  MOZ_ASSERT(!arenasToSweep(thingKind));
+
+  arenasToSweep(thingKind) = arenaList(thingKind).head();
+  arenaList(thingKind).clear();
 }
 
 void ArenaLists::queueForBackgroundSweep(JSFreeOp* fop,
                                          const FinalizePhase& phase) {
   gcstats::AutoPhase ap(fop->runtime()->gc.stats(), phase.statsPhase);
   for (auto kind : phase.kinds) {
     queueForBackgroundSweep(kind);
   }
 }
 
 inline void ArenaLists::queueForBackgroundSweep(AllocKind thingKind) {
   MOZ_ASSERT(IsBackgroundFinalized(thingKind));
 
-  ArenaList* al = &arenaLists(thingKind);
+  ArenaList* al = &arenaList(thingKind);
   if (al->isEmpty()) {
     MOZ_ASSERT(concurrentUse(thingKind) == ConcurrentUse::None);
     return;
   }
 
   MOZ_ASSERT(concurrentUse(thingKind) == ConcurrentUse::None);
 
-  arenaListsToSweep(thingKind) = al->head();
+  arenasToSweep(thingKind) = al->head();
   al->clear();
   concurrentUse(thingKind) = ConcurrentUse::BackgroundFinalize;
 }
 
 /*static*/
 void ArenaLists::backgroundFinalize(JSFreeOp* fop, Arena* listHead,
                                     Arena** empty) {
   MOZ_ASSERT(listHead);
@@ -2765,22 +2765,22 @@ void ArenaLists::backgroundFinalize(JSFr
   SortedArenaList finalizedSorted(thingsPerArena);
 
   auto unlimited = SliceBudget::unlimited();
   FinalizeArenas(fop, &listHead, finalizedSorted, thingKind, unlimited);
   MOZ_ASSERT(!listHead);
 
   finalizedSorted.extractEmpty(empty);
 
-  // When arenas are queued for background finalization, all arenas are moved
-  // to arenaListsToSweep[], leaving the arenaLists[] empty. However, new
-  // arenas may be allocated before background finalization finishes; now that
-  // finalization is complete, we want to merge these lists back together.
+  // When arenas are queued for background finalization, all arenas are moved to
+  // arenasToSweep, leaving the arena list empty. However, new arenas may be
+  // allocated before background finalization finishes; now that finalization is
+  // complete, we want to merge these lists back together.
   ArenaLists* lists = &zone->arenas;
-  ArenaList* al = &lists->arenaLists(thingKind);
+  ArenaList* al = &lists->arenaList(thingKind);
 
   // Flatten |finalizedSorted| into a regular ArenaList.
   ArenaList finalized = finalizedSorted.toArenaList();
 
   // We must take the GC lock to be able to safely modify the ArenaList;
   // however, this does not by itself make the changes visible to all threads,
   // as not all threads take the GC lock to read the ArenaLists.
   // That safety is provided by the ReleaseAcquire memory ordering of the
@@ -2788,45 +2788,45 @@ void ArenaLists::backgroundFinalize(JSFr
   {
     AutoLockGC lock(lists->runtimeFromAnyThread());
     MOZ_ASSERT(lists->concurrentUse(thingKind) ==
                ConcurrentUse::BackgroundFinalize);
 
     // Join |al| and |finalized| into a single list.
     *al = finalized.insertListWithCursorAtEnd(*al);
 
-    lists->arenaListsToSweep(thingKind) = nullptr;
+    lists->arenasToSweep(thingKind) = nullptr;
   }
 
   lists->concurrentUse(thingKind) = ConcurrentUse::None;
 }
 
 void ArenaLists::releaseForegroundSweptEmptyArenas() {
   AutoLockGC lock(runtime());
   ReleaseArenaList(runtime(), savedEmptyArenas, lock);
   savedEmptyArenas = nullptr;
 }
 
 void ArenaLists::queueForegroundThingsForSweep() {
-  gcShapeArenasToUpdate = arenaListsToSweep(AllocKind::SHAPE);
-  gcAccessorShapeArenasToUpdate = arenaListsToSweep(AllocKind::ACCESSOR_SHAPE);
-  gcObjectGroupArenasToUpdate = arenaListsToSweep(AllocKind::OBJECT_GROUP);
-  gcScriptArenasToUpdate = arenaListsToSweep(AllocKind::SCRIPT);
+  gcShapeArenasToUpdate = arenasToSweep(AllocKind::SHAPE);
+  gcAccessorShapeArenasToUpdate = arenasToSweep(AllocKind::ACCESSOR_SHAPE);
+  gcObjectGroupArenasToUpdate = arenasToSweep(AllocKind::OBJECT_GROUP);
+  gcScriptArenasToUpdate = arenasToSweep(AllocKind::SCRIPT);
 }
 
 void ArenaLists::checkSweepStateNotInUse() {
   // Called before and after sweeping to check the sweep state is as expected.
 #ifdef DEBUG
   checkNoArenasToUpdate();
   MOZ_ASSERT(incrementalSweptArenaKind == AllocKind::LIMIT);
   MOZ_ASSERT(incrementalSweptArenas.ref().isEmpty());
   MOZ_ASSERT(!savedEmptyArenas);
   for (auto i : AllAllocKinds()) {
     MOZ_ASSERT(concurrentUse(i) == ConcurrentUse::None);
-    MOZ_ASSERT(!arenaListsToSweep(i));
+    MOZ_ASSERT(!arenasToSweep(i));
   }
 #endif
 }
 
 void ArenaLists::checkNoArenasToUpdate() {
   MOZ_ASSERT(!gcShapeArenasToUpdate);
   MOZ_ASSERT(!gcAccessorShapeArenasToUpdate);
   MOZ_ASSERT(!gcScriptArenasToUpdate);
@@ -3297,17 +3297,17 @@ void GCRuntime::sweepBackgroundThings(Zo
     Arena* emptyArenas = nullptr;
 
     AutoSetThreadIsSweeping threadIsSweeping(zone);
 
     // We must finalize thing kinds in the order specified by
     // BackgroundFinalizePhases.
     for (auto phase : BackgroundFinalizePhases) {
       for (auto kind : phase.kinds) {
-        Arena* arenas = zone->arenas.arenaListsToSweep(kind);
+        Arena* arenas = zone->arenas.arenasToSweep(kind);
         MOZ_RELEASE_ASSERT(uintptr_t(arenas) != uintptr_t(-1));
         if (arenas) {
           ArenaLists::backgroundFinalize(&fop, arenas, &emptyArenas);
         }
       }
     }
 
     // Release any arenas that are now empty.
@@ -3334,17 +3334,17 @@ void GCRuntime::assertBackgroundSweeping
 #ifdef DEBUG
   {
     AutoLockHelperThreadState lock;
     MOZ_ASSERT(backgroundSweepZones.ref().isEmpty());
   }
 
   for (ZonesIter zone(this, WithAtoms); !zone.done(); zone.next()) {
     for (auto i : AllAllocKinds()) {
-      MOZ_ASSERT(!zone->arenas.arenaListsToSweep(i));
+      MOZ_ASSERT(!zone->arenas.arenasToSweep(i));
       MOZ_ASSERT(zone->arenas.doneBackgroundFinalize(i));
     }
   }
 #endif
 }
 
 void GCRuntime::queueZonesAndStartBackgroundSweep(ZoneList& zones) {
   {
@@ -3629,17 +3629,17 @@ void GCRuntime::sweepZones(JSFreeOp* fop
       zone->sweepCompartments(fop, true, destroyingRuntime);
     }
     *write++ = zone;
   }
   zones().shrinkTo(write - zones().begin());
 }
 
 void ArenaLists::checkEmptyArenaList(AllocKind kind) {
-  MOZ_ASSERT(arenaLists(kind).isEmpty());
+  MOZ_ASSERT(arenaList(kind).isEmpty());
 }
 
 class MOZ_RAII AutoRunParallelTask : public GCParallelTask {
   // This class takes a pointer to a member function of GCRuntime.
   using TaskFunc = JS_MEMBER_FN_PTR_TYPE(GCRuntime, void);
 
   TaskFunc func_;
   gcstats::PhaseKind phase_;
@@ -3885,17 +3885,17 @@ static bool ShouldCollectZone(Zone* zone
 bool GCRuntime::prepareZonesForCollection(JS::GCReason reason,
                                           bool* isFullOut) {
 #ifdef DEBUG
   /* Assert that zone state is as we expect */
   for (ZonesIter zone(this, WithAtoms); !zone.done(); zone.next()) {
     MOZ_ASSERT(!zone->isCollecting());
     MOZ_ASSERT_IF(!zone->isAtomsZone(), !zone->compartments().empty());
     for (auto i : AllAllocKinds()) {
-      MOZ_ASSERT(!zone->arenas.arenaListsToSweep(i));
+      MOZ_ASSERT(!zone->arenas.arenasToSweep(i));
     }
   }
 #endif
 
   *isFullOut = true;
   bool any = false;
 
   auto currentTime = ReallyNow();
@@ -5481,43 +5481,43 @@ void GCRuntime::beginSweepPhase(JS::GCRe
   sweepActions->assertFinished();
 }
 
 bool ArenaLists::foregroundFinalize(JSFreeOp* fop, AllocKind thingKind,
                                     SliceBudget& sliceBudget,
                                     SortedArenaList& sweepList) {
   checkNoArenasToUpdateForKind(thingKind);
 
-  if (!arenaListsToSweep(thingKind) && incrementalSweptArenas.ref().isEmpty()) {
+  if (!arenasToSweep(thingKind) && incrementalSweptArenas.ref().isEmpty()) {
     return true;
   }
 
   // Arenas are released for use for new allocations as soon as the finalizers
   // for that allocation kind have run. This means that a cell's finalizer can
   // safely use IsAboutToBeFinalized to check other cells of the same alloc
   // kind, but not of different alloc kinds: the other arena may have already
   // had new objects allocated in it, and since we allocate black,
   // IsAboutToBeFinalized will return false even though the referent we intended
   // to check is long gone.
-  if (!FinalizeArenas(fop, &arenaListsToSweep(thingKind), sweepList, thingKind,
+  if (!FinalizeArenas(fop, &arenasToSweep(thingKind), sweepList, thingKind,
                       sliceBudget)) {
     incrementalSweptArenaKind = thingKind;
     incrementalSweptArenas = sweepList.toArenaList();
     return false;
   }
 
   // Clear any previous incremental sweep state we may have saved.
   incrementalSweptArenaKind = AllocKind::LIMIT;
   incrementalSweptArenas.ref().clear();
 
   sweepList.extractEmpty(&savedEmptyArenas.ref());
 
   ArenaList finalized = sweepList.toArenaList();
-  arenaLists(thingKind) =
-      finalized.insertListWithCursorAtEnd(arenaLists(thingKind));
+  arenaList(thingKind) =
+      finalized.insertListWithCursorAtEnd(arenaList(thingKind));
 
   return true;
 }
 
 void js::gc::SweepMarkTask::run() {
   // Time reporting is handled separately for parallel tasks.
   gc->sweepMarkResult =
       gc->markUntilBudgetExhausted(this->budget, GCMarker::DontReportMarkTime);
@@ -6209,17 +6209,17 @@ void GCRuntime::endSweepPhase(bool destr
 #ifdef JS_GC_ZEAL
   finishMarkingValidation();
 #endif
 
 #ifdef DEBUG
   for (ZonesIter zone(this, WithAtoms); !zone.done(); zone.next()) {
     for (auto i : AllAllocKinds()) {
       MOZ_ASSERT_IF(!IsBackgroundFinalized(i) || !sweepOnBackgroundThread,
-                    !zone->arenas.arenaListsToSweep(i));
+                    !zone->arenas.arenasToSweep(i));
     }
   }
 #endif
 
   AssertNoWrappersInGrayList(rt);
 }
 
 void GCRuntime::beginCompactPhase() {
@@ -7951,18 +7951,18 @@ void ArenaLists::adoptArenas(ArenaLists*
                              bool targetZoneIsCollecting) {
   // GC may be active so take the lock here so we can mutate the arena lists.
   AutoLockGC lock(runtime());
 
   fromArenaLists->clearFreeLists();
 
   for (auto thingKind : AllAllocKinds()) {
     MOZ_ASSERT(fromArenaLists->concurrentUse(thingKind) == ConcurrentUse::None);
-    ArenaList* fromList = &fromArenaLists->arenaLists(thingKind);
-    ArenaList* toList = &arenaLists(thingKind);
+    ArenaList* fromList = &fromArenaLists->arenaList(thingKind);
+    ArenaList* toList = &arenaList(thingKind);
     fromList->check();
     toList->check();
     Arena* next;
     for (Arena* fromArena = fromList->head(); fromArena; fromArena = next) {
       // Copy fromArena->next before releasing/reinserting.
       next = fromArena->next;
 
       MOZ_ASSERT(!fromArena->isEmpty());