Closed Bug 573175 Opened 15 years ago Closed 15 years ago

TM: run the cycle collector without forcing a JS GC

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Version:
Other Branch
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: gal, Assigned: gal)

References

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 1 obsolete file)

patch
24.14 KB, patch
jst
: review+
Details | Diff | Splinter Review
When doing a cycle collection, only for a JS GC on shutdown. Otherwise use the gray/white heap analysis the JS GC did the last time it ran.
Depends on: 573060
Blocks: 565217
Attached patch patch (obsolete) — — Splinter Review
Untested.
Assignee: general → gal
A partially working patch in a second. We are touching dead objects, so something is still off. cc: Starting nsCycleCollector::Collect(1) ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 ###!!! ASSERTION: WrappedNativeSuspecter attempting to touch dead object: '!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject())', file ../../../../../js/src/xpconnect/src/xpcwrappednativescope.cpp, line 419 cc: mRuntimes[*]->BeginCycleCollection() took 3ms cc: SelectPurple() took 0ms cc: MarkRoots() took 10ms cc: ScanRoots() took 1ms cc: RootWhite() took 0ms cc: CollectWhite() took 0ms cc: Collect() took 18ms
We are suspecting wrappers that are dead since the last GC. I think we are not purging these properly after a GC. Need to find Blake. static JSDHashOperator WrappedNativeSuspecter(JSDHashTable *table, JSDHashEntryHdr *hdr, uint32 number, void *arg) { SuspectClosure* closure = static_cast<SuspectClosure*>(arg); XPCWrappedNative* wrapper = ((Native2WrappedNativeMap::Entry*)hdr)->value; if(wrapper->IsValid() && wrapper->HasExternalReference() && !wrapper->IsWrapperExpired()) { NS_ASSERTION(NS_IsMainThread(), "Suspecting wrapped natives from non-main thread"); NS_ASSERTION(!JS_IsAboutToBeFinalized(closure->cx, wrapper->GetFlatJSObject()), "WrappedNativeSuspecter attempting to touch dead object");
Attached patch patch — — Splinter Review
Attachment #452383 - Attachment is obsolete: true
Comment on attachment 453315 [details] [diff] [review] patch This patch finishes the split between CC and GC. The CC no longer forces a GC, except if a) its the first CC (we need the mark bits to be valid), or b) its the shutdown dance (we CC in circles, we need the GC to run in between). I made the heuristics in nsJSEnvironment call the GC and then the CC to be compatible with what we do right now, so this patch has no observable effect. However, with this in place GC and CC can be split now (to play with it just remove the GC call and you will see the browser accumulate garbage until the next GC is forced by the engine itself). This patch should be followed up by a patch with a better heuristics that spreads out GCs and CCs.
Attachment #453315 - Flags: review?(peterv)
Review ping.
Jesse, after this patch DOMUtils::CC() will no longer trigger TWO CCs. It will only trigger one. And DOMUtils:GC() will trigger ONE GC. You can call these functions repeatedly if you want more than one round.
I think GC and CC should hang off Components.utils rather than nsDOMWindowUtils. I don't always have a DOM window handy.
Easy to do. Want to file a bug for that?
Hmm, there's already a Components.utils.forceGC, but you're not touching that?
What the hell is that?
Than one does: JS_GC(cx); I wonder why we have 12 APIs for this. Any idea?
Review ping =)
Blocks: 572542
Comment on attachment 453315 [details] [diff] [review] patch Stealing review. Peterv, feel free to look after the fact if you feel like it. - In nsXPConnect::BeginCycleCollection(): + mCycleCollectionContext = new XPCCallContext(NATIVE_CALLER); + if (!mCycleCollectionContext || !mCycleCollectionContext->IsValid()) { No need to null check mCycleCollectionContext there, new is infallible now. diff --git a/xpcom/base/nsCycleCollector.cpp b/xpcom/base/nsCycleCollector.cpp --- a/xpcom/base/nsCycleCollector.cpp +++ b/xpcom/base/nsCycleCollector.cpp @@ -1,9 +1,9 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ + /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ Undo this. nsCycleCollector::nsCycleCollector() : mCollectionInProgress(PR_FALSE), mScanInProgress(PR_FALSE), mCollectedObjects(0), + mFirstCollection(true), PR_TRUE, for consistency's sake. - In xpcom/base/nsCycleCollector.h: struct nsCycleCollectionJSRuntime : public nsCycleCollectionLanguageRuntime { /** - * Runs cycle collection and returns whether cycle collection collected - * anything. + * Runs the JavaScript GC. */ - virtual PRBool Collect() = 0; + virtual void Collect() = 0; 4-space indentation. r=jst with that.
Attachment #453315 - Flags: review?(peterv) → review+
http://hg.mozilla.org/tracemonkey/rev/cd90ffb63d61
Whiteboard: fixed-in-tracemonkey
Comment on attachment 453315 [details] [diff] [review] patch >diff --git a/js/src/xpconnect/idl/nsIXPConnect.idl b/js/src/xpconnect/idl/nsIXPConnect.idl >+ void GarbageCollect(); XPIDL uses lowercase for the first letter. >+nsXPConnect::GarbageCollect() >+{ >+ Collect(); >+ return NS_OK; Why didn't you just rename nsXPConnect::Collect to nsXPConnect::GarbageCollect instead of adding a new function? It'd make more sense too for Collect to be named GarbageCollect, since all it does is call JS_GC. > nsXPConnect::GetRequestDepth(JSContext* cx) > { > PRInt32 requestDepth = cx->outstandingRequests; >- XPCCallContext* context = GetCycleCollectionContext(); >+ XPCCallContext* context = mCycleCollectionContext; >+ // Ignore the request from the XPCCallContext we created for cycle >+ // collection. > if(context && cx == context->GetJSContext()) There's really no need for |context|, could just use |mCycleCollectionContext| directly. >+ PRBool collected = BeginCollection() && FinishCollection(); I filed bug 584048 on merging BeginCollection and FinishCollection.
http://hg.mozilla.org/mozilla-central/rev/cd90ffb63d61
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: