From 3c4ae81d155711983cc893c9634ce87842044648 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 7 May 2014 23:07:33 -0700 Subject: [PATCH] Bug 1006300 - Encapsulate and add better documentation and checking for ArenaList. r=jonco. --HG-- extra : rebase_source : b05df6c5e6ac2a501f2f07a9c57cbba75caadaf1 --- js/src/gc/Heap.h | 2 +- js/src/jsgc.cpp | 108 +++++++++++---------------- js/src/jsgc.h | 188 ++++++++++++++++++++++++++++++++++++++++------- 3 files changed, 202 insertions(+), 96 deletions(-) diff --git a/js/src/gc/Heap.h b/js/src/gc/Heap.h index 4aa7cff67650..5662560afabd 100644 --- a/js/src/gc/Heap.h +++ b/js/src/gc/Heap.h @@ -38,7 +38,7 @@ class FreeOp; namespace gc { struct Arena; -struct ArenaList; +class ArenaList; struct ArenaHeader; struct Chunk; diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index ac2859d38140..786476e4f928 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -498,20 +498,6 @@ Arena::finalize(FreeOp *fop, AllocKind thingKind, size_t thingSize) return false; } -/* - * Insert an arena into the list in appropriate position and update the cursor - * to ensure that any arena before the cursor is full. - */ -void ArenaList::insert(ArenaHeader *a) -{ - JS_ASSERT(a); - JS_ASSERT_IF(!head, cursor == &head); - a->next = *cursor; - *cursor = a; - if (!a->hasFreeThings()) - cursor = &a->next; -} - template static inline bool FinalizeTypedArenas(FreeOp *fop, @@ -538,7 +524,7 @@ FinalizeTypedArenas(FreeOp *fop, *src = aheader->next; bool allClear = aheader->getArena()->finalize(fop, thingKind, thingSize); if (!allClear) - dest.insert(aheader); + dest.insertAtCursor(aheader); else if (releaseArenas) aheader->chunk()->releaseArena(aheader); else @@ -548,13 +534,14 @@ FinalizeTypedArenas(FreeOp *fop, if (budget.isOverBudget()) return false; } + dest.deepCheck(); return true; } /* - * Finalize the list. On return al->cursor points to the first non-empty arena - * after the al->head. + * Finalize the list. On return, |al|'s cursor points to the first non-empty + * arena in the list (which may be null if all arenas are full). */ static bool FinalizeArenas(FreeOp *fop, @@ -563,7 +550,7 @@ FinalizeArenas(FreeOp *fop, AllocKind thingKind, SliceBudget &budget) { - switch(thingKind) { + switch (thingKind) { case FINALIZE_OBJECT0: case FINALIZE_OBJECT0_BACKGROUND: case FINALIZE_OBJECT2: @@ -935,7 +922,7 @@ void Chunk::recycleArena(ArenaHeader *aheader, ArenaList &dest, AllocKind thingKind) { aheader->getArena()->setAsFullyUnused(thingKind); - dest.insert(aheader); + dest.insertAtCursor(aheader); } void @@ -1484,13 +1471,13 @@ ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind) volatile uintptr_t *bfs = &backgroundFinalizeState[thingKind]; if (*bfs != BFS_DONE) { /* - * We cannot search the arena list for free things while the - * background finalization runs and can modify head or cursor at any - * moment. So we always allocate a new arena in that case. + * We cannot search the arena list for free things while background + * finalization runs and can modify it at any moment. So we always + * allocate a new arena in that case. */ maybeLock.lock(zone->runtimeFromAnyThread()); if (*bfs == BFS_RUN) { - JS_ASSERT(!*al->cursor); + JS_ASSERT(al->isCursorAtEnd()); chunk = PickChunk(zone); if (!chunk) { /* @@ -1509,9 +1496,7 @@ ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind) #endif /* JS_THREADSAFE */ if (!chunk) { - if (ArenaHeader *aheader = *al->cursor) { - JS_ASSERT(aheader->hasFreeThings()); - + if (ArenaHeader *aheader = al->arenaAfterCursor()) { /* * Normally, the empty arenas are returned to the chunk * and should not present on the list. In parallel @@ -1519,7 +1504,8 @@ ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind) * list to avoid synchronizing on the chunk. */ JS_ASSERT(!aheader->isEmpty() || InParallelSection()); - al->cursor = &aheader->next; + + al->moveCursorPast(aheader); /* * Move the free span stored in the arena to the free list and @@ -1551,11 +1537,11 @@ ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind) * as full as its single free span is moved to the free lits, and insert * it to the list as a fully allocated arena. * - * We add the arena before the the head, not after the tail pointed by the - * cursor, so after the GC the most recently added arena will be used first - * for allocations improving cache locality. + * We add the arena before the the head, so that after the GC the most + * recently added arena will be used first for allocations. This improves + * cache locality. */ - JS_ASSERT(!*al->cursor); + JS_ASSERT(al->isCursorAtEnd()); ArenaHeader *aheader = chunk->allocateArena(zone, thingKind); if (!aheader) return nullptr; @@ -1568,12 +1554,7 @@ ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind) PushArenaAllocatedDuringSweep(zone->runtimeFromMainThread(), aheader); } } - aheader->next = al->head; - if (!al->head) { - JS_ASSERT(al->cursor == &al->head); - al->cursor = &aheader->next; - } - al->head = aheader; + al->insertAtStart(aheader); /* See comments before allocateFromNewArena about this assert. */ JS_ASSERT(!aheader->hasFreeThings()); @@ -1604,7 +1585,7 @@ ArenaLists::wipeDuringParallelExecution(JSRuntime *rt) // that'd be bad. for (unsigned i = 0; i < FINALIZE_LAST; i++) { AllocKind thingKind = AllocKind(i); - if (!IsBackgroundFinalized(thingKind) && arenaLists[thingKind].head) + if (!IsBackgroundFinalized(thingKind) && !arenaLists[thingKind].isEmpty()) return; } @@ -1617,7 +1598,7 @@ ArenaLists::wipeDuringParallelExecution(JSRuntime *rt) if (!IsBackgroundFinalized(thingKind)) continue; - if (arenaLists[i].head) { + if (!arenaLists[i].isEmpty()) { purge(thingKind); forceFinalizeNow(&fop, thingKind); } @@ -1636,7 +1617,7 @@ ArenaLists::forceFinalizeNow(FreeOp *fop, AllocKind thingKind) { JS_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE); - ArenaHeader *arenas = arenaLists[thingKind].head; + ArenaHeader *arenas = arenaLists[thingKind].head(); arenaLists[thingKind].clear(); SliceBudget budget; @@ -1651,7 +1632,7 @@ ArenaLists::queueForForegroundSweep(FreeOp *fop, AllocKind thingKind) JS_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE); JS_ASSERT(!arenaListsToSweep[thingKind]); - arenaListsToSweep[thingKind] = arenaLists[thingKind].head; + arenaListsToSweep[thingKind] = arenaLists[thingKind].head(); arenaLists[thingKind].clear(); } @@ -1665,9 +1646,8 @@ ArenaLists::queueForBackgroundSweep(FreeOp *fop, AllocKind thingKind) #endif ArenaList *al = &arenaLists[thingKind]; - if (!al->head) { + if (al->isEmpty()) { JS_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE); - JS_ASSERT(al->cursor == &al->head); return; } @@ -1678,7 +1658,7 @@ ArenaLists::queueForBackgroundSweep(FreeOp *fop, AllocKind thingKind) JS_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE || backgroundFinalizeState[thingKind] == BFS_JUST_FINISHED); - arenaListsToSweep[thingKind] = al->head; + arenaListsToSweep[thingKind] = al->head(); al->clear(); backgroundFinalizeState[thingKind] = BFS_RUN; } @@ -1695,23 +1675,18 @@ ArenaLists::backgroundFinalize(FreeOp *fop, ArenaHeader *listHead, bool onBackgr FinalizeArenas(fop, &listHead, finalized, thingKind, budget); JS_ASSERT(!listHead); - /* - * After we finish the finalization al->cursor must point to the end of - * the head list as we emptied the list before the background finalization - * and the allocation adds new arenas before the cursor. - */ + // When arenas are queued for background finalization, all + // arenas are moved to arenaListsToSweep[], leaving the arenaLists[] empty. + // Then, if new arenas are allocated before background finalization + // finishes they are always added to the front of the list. Therefore, + // at this point, |al|'s cursor will always be at the end of its list. ArenaLists *lists = &zone->allocator.arenas; ArenaList *al = &lists->arenaLists[thingKind]; AutoLockGC lock(fop->runtime()); JS_ASSERT(lists->backgroundFinalizeState[thingKind] == BFS_RUN); - JS_ASSERT(!*al->cursor); - if (finalized.head) { - *al->cursor = finalized.head; - if (finalized.cursor != &finalized.head) - al->cursor = finalized.cursor; - } + al->appendToListWithCursorAtEnd(finalized); /* * We must set the state to BFS_JUST_FINISHED if we are running on the @@ -1722,7 +1697,7 @@ ArenaLists::backgroundFinalize(FreeOp *fop, ArenaHeader *listHead, bool onBackgr * allocating new arenas from the chunks we can set the state to BFS_DONE if * we have released all finalized arenas back to their chunks. */ - if (onBackgroundThread && finalized.head) + if (onBackgroundThread && !finalized.isEmpty()) lists->backgroundFinalizeState[thingKind] = BFS_JUST_FINISHED; else lists->backgroundFinalizeState[thingKind] = BFS_DONE; @@ -5335,11 +5310,12 @@ ArenaLists::adoptArenas(JSRuntime *rt, ArenaLists *fromArenaLists) #endif ArenaList *fromList = &fromArenaLists->arenaLists[thingKind]; ArenaList *toList = &arenaLists[thingKind]; - while (fromList->head != nullptr) { - // Remove entry from |fromList| - ArenaHeader *fromHeader = fromList->head; - fromList->head = fromHeader->next; - fromHeader->next = nullptr; + fromList->deepCheck(); + toList->deepCheck(); + ArenaHeader *next; + for (ArenaHeader *fromHeader = fromList->head(); fromHeader; fromHeader = next) { + // Copy fromHeader->next before releasing/reinserting. + next = fromHeader->next; // During parallel execution, we sometimes keep empty arenas // on the lists rather than sending them back to the chunk. @@ -5348,9 +5324,10 @@ ArenaLists::adoptArenas(JSRuntime *rt, ArenaLists *fromArenaLists) if (fromHeader->isEmpty()) fromHeader->chunk()->releaseArena(fromHeader); else - toList->insert(fromHeader); + toList->insertAtCursor(fromHeader); } - fromList->cursor = &fromList->head; + fromList->clear(); + toList->deepCheck(); } } @@ -5359,10 +5336,7 @@ ArenaLists::containsArena(JSRuntime *rt, ArenaHeader *needle) { AutoLockGC lock(rt); size_t allocKind = needle->getAllocKind(); - for (ArenaHeader *aheader = arenaLists[allocKind].head; - aheader != nullptr; - aheader = aheader->next) - { + for (ArenaHeader *aheader = arenaLists[allocKind].head(); aheader; aheader = aheader->next) { if (aheader == needle) return true; } diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 96e6ab1c5259..97c391a674f9 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -365,30 +365,165 @@ GetGCKindSlots(AllocKind thingKind, const Class *clasp) } /* - * ArenaList::head points to the start of the list. Normally cursor points - * to the first arena in the list with some free things and all arenas - * before cursor are fully allocated. However, as the arena currently being - * allocated from is considered full while its list of free spans is moved - * into the freeList, during the GC or cell enumeration, when an - * unallocated freeList is moved back to the arena, we can see an arena - * with some free cells before the cursor. The cursor is an indirect - * pointer to allow for efficient list insertion at the cursor point and - * other list manipulations. + * Arena lists have a head and a cursor. The cursor conceptually lies on arena + * boundaries, i.e. before the first arena, between two arenas, or after the + * last arena. + * + * Normally the arena following the cursor is the first arena in the list with + * some free things and all arenas before the cursor are fully allocated. (And + * if the cursor is at the end of the list, then all the arenas are full.) + * + * However, the arena currently being allocated from is considered full while + * its list of free spans is moved into the freeList. Therefore, during GC or + * cell enumeration, when an unallocated freeList is moved back to the arena, + * we can see an arena with some free cells before the cursor. + * + * Arenas following the cursor should not be full. */ -struct ArenaList { - ArenaHeader *head; - ArenaHeader **cursor; +class ArenaList { + // The cursor is implemented via an indirect pointer, |cursorp_|, to allow + // for efficient list insertion at the cursor point and other list + // manipulations. + // + // - If the list is empty: |head| is null, |cursorp_| points to |head|, and + // therefore |*cursorp_| is null. + // + // - If the list is not empty: |head| is non-null, and... + // + // - If the cursor is at the start of the list: |cursorp_| points to + // |head|, and therefore |*cursorp_| points to the first arena. + // + // - If cursor is at the end of the list: |cursorp_| points to the |next| + // field of the last arena, and therefore |*cursorp_| is null. + // + // - If the cursor is at neither the start nor the end of the list: + // |cursorp_| points to the |next| field of the arena preceding the + // cursor, and therefore |*cursorp_| points to the arena following the + // cursor. + // + // |cursorp_| is never null. + // + ArenaHeader *head_; + ArenaHeader **cursorp_; + public: ArenaList() { clear(); } - void clear() { - head = nullptr; - cursor = &head; + // This does checking just of |head_| and |cursorp_|. + void check() const { +#ifdef DEBUG + // If the list is empty, it must have this form. + JS_ASSERT_IF(!head_, cursorp_ == &head_); + + // If there's an arena following the cursor, it must not be full. + ArenaHeader *cursor = *cursorp_; + JS_ASSERT_IF(cursor, cursor->hasFreeThings()); +#endif } - void insert(ArenaHeader *arena); + // This does checking involving all the arenas in the list. + void deepCheck() const { +#ifdef DEBUG + check(); + // All full arenas must precede all non-full arenas. + // + // XXX: this is currently commented out because it fails moderately + // often. I'm not sure if this is because (a) it's not true that all + // full arenas must precede all non-full arenas, or (b) we have some + // defective list-handling code. + // +// bool havePassedFullArenas = false; +// for (ArenaHeader *aheader = head_; aheader; aheader = aheader->next) { +// if (havePassedFullArenas) { +// JS_ASSERT(aheader->hasFreeThings()); +// } else if (aheader->hasFreeThings()) { +// havePassedFullArenas = true; +// } +// } +#endif + } + + void clear() { + head_ = nullptr; + cursorp_ = &head_; + check(); + } + + bool isEmpty() const { + check(); + return !head_; + } + + // This returns nullptr if the list is empty. + ArenaHeader *head() const { + check(); + return head_; + } + + bool isCursorAtEnd() const { + check(); + return !*cursorp_; + } + + // This can return nullptr. + ArenaHeader *arenaAfterCursor() const { + check(); + return *cursorp_; + } + + // This moves the cursor past |aheader|. |aheader| must be an arena within + // this list. + void moveCursorPast(ArenaHeader *aheader) { + cursorp_ = &aheader->next; + check(); + } + + // This does two things. + // - Inserts |a| at the cursor. + // - Leaves the cursor sitting just before |a|, if |a| is not full, or just + // after |a|, if |a| is full. + // + void insertAtCursor(ArenaHeader *a) { + check(); + a->next = *cursorp_; + *cursorp_ = a; + // At this point, the cursor is sitting before |a|. Move it after |a| + // if necessary. + if (!a->hasFreeThings()) + cursorp_ = &a->next; + check(); + } + + // This inserts |a| at the start of the list, and doesn't change the + // cursor. + void insertAtStart(ArenaHeader *a) { + check(); + a->next = head_; + if (isEmpty()) + cursorp_ = &a->next; // The cursor remains null. + head_ = a; + check(); + } + + // Appends |list|. |this|'s cursor must be at the end. + void appendToListWithCursorAtEnd(ArenaList &other) { + JS_ASSERT(isCursorAtEnd()); + deepCheck(); + other.deepCheck(); + if (!other.isEmpty()) { + // Because |this|'s cursor is at the end, |cursorp_| points to the + // list-ending null. So this assignment appends |other| to |this|. + *cursorp_ = other.head_; + + // If |other|'s cursor isn't at the start of the list, then update + // |this|'s cursor accordingly. + if (other.cursorp_ != &other.head_) + cursorp_ = other.cursorp_; + } + deepCheck(); + } }; class ArenaLists @@ -408,7 +543,7 @@ class ArenaLists /* * The background finalization adds the finalized arenas to the list at - * the *cursor position. backgroundFinalizeState controls the interaction + * the cursor position. backgroundFinalizeState controls the interaction * between the GC lock and the access to the list from the allocation * thread. * @@ -420,7 +555,7 @@ class ArenaLists * lock. The former indicates that the finalization still runs. The latter * signals that finalization just added to the list finalized arenas. In * that case the lock effectively serves as a read barrier to ensure that - * the allocation thread see all the writes done during finalization. + * the allocation thread sees all the writes done during finalization. */ enum BackgroundFinalizeState { BFS_DONE, @@ -455,9 +590,10 @@ class ArenaLists * the background finalization is disabled. */ JS_ASSERT(backgroundFinalizeState[i] == BFS_DONE); - ArenaHeader **headp = &arenaLists[i].head; - while (ArenaHeader *aheader = *headp) { - *headp = aheader->next; + ArenaHeader *next; + for (ArenaHeader *aheader = arenaLists[i].head(); aheader; aheader = next) { + // Copy aheader->next before releasing. + next = aheader->next; aheader->chunk()->releaseArena(aheader); } } @@ -473,7 +609,7 @@ class ArenaLists } ArenaHeader *getFirstArena(AllocKind thingKind) const { - return arenaLists[thingKind].head; + return arenaLists[thingKind].head(); } ArenaHeader *getFirstArenaToSweep(AllocKind thingKind) const { @@ -488,22 +624,18 @@ class ArenaLists */ if (backgroundFinalizeState[i] != BFS_DONE) return false; - if (arenaLists[i].head) + if (!arenaLists[i].isEmpty()) return false; } return true; } - bool arenasAreFull(AllocKind thingKind) const { - return !*arenaLists[thingKind].cursor; - } - void unmarkAll() { for (size_t i = 0; i != FINALIZE_LIMIT; ++i) { /* The background finalization must have stopped at this point. */ JS_ASSERT(backgroundFinalizeState[i] == BFS_DONE || backgroundFinalizeState[i] == BFS_JUST_FINISHED); - for (ArenaHeader *aheader = arenaLists[i].head; aheader; aheader = aheader->next) { + for (ArenaHeader *aheader = arenaLists[i].head(); aheader; aheader = aheader->next) { uintptr_t *word = aheader->chunk()->bitmap.arenaBits(aheader); memset(word, 0, ArenaBitmapWords * sizeof(uintptr_t)); }