From 4ffd9a54064f826051e0eedd91d422144a59eb44 Mon Sep 17 00:00:00 2001 From: "igor@mir2.org" Date: Sat, 5 Jan 2008 17:20:16 -0800 Subject: [PATCH] Bug 409109: Backing out once again to figure out the reason for talos regressions. --- dom/src/base/nsJSEnvironment.cpp | 101 +++++++++++++------------ dom/src/base/nsJSEnvironment.h | 6 +- js/src/jsapi.h | 13 +--- js/src/jscntxt.h | 4 +- js/src/xpconnect/src/xpccomponents.cpp | 39 ++++------ 5 files changed, 78 insertions(+), 85 deletions(-) diff --git a/dom/src/base/nsJSEnvironment.cpp b/dom/src/base/nsJSEnvironment.cpp index 4324c9b66f9..a59ecbfb51b 100644 --- a/dom/src/base/nsJSEnvironment.cpp +++ b/dom/src/base/nsJSEnvironment.cpp @@ -782,17 +782,19 @@ PrintWinCodebase(nsGlobalWindow *win) } #endif -// The operation limit before we even check if our start timestamp is -// initialized. This is a fairly low number as we want to initialize the -// timestamp early enough to not waste much time before we get there, but we -// don't want to bother doing this too early as it's not generally necessary. -const PRUint32 INITIALIZE_TIME_OPERATION_LIMIT = 256 * JS_OPERATION_WEIGHT_BASE; +// The number of branch callbacks between calls to JS_MaybeGC +#define MAYBE_GC_BRANCH_COUNT_MASK 0x00000fff // 4095 -// The operation limit between calls to JS_MaybeGC -const PRUint32 MAYBE_GC_OPERATION_LIMIT = INITIALIZE_TIME_OPERATION_LIMIT * 16; +// The number of branch callbacks before we even check if our start +// timestamp is initialized. This is a fairly low number as we want to +// initialize the timestamp early enough to not waste much time before +// we get there, but we don't want to bother doing this too early as +// it's not generally necessary. +#define INITIALIZE_TIME_BRANCH_COUNT_MASK 0x000000ff // 255 +// This function is called after each JS branch execution JSBool JS_DLL_CALLBACK -nsJSContext::DOMOperationCallback(JSContext *cx) +nsJSContext::DOMBranchCallback(JSContext *cx, JSScript *script) { // Get the native context nsJSContext *ctx = static_cast(::JS_GetContextPrivate(cx)); @@ -802,34 +804,40 @@ nsJSContext::DOMOperationCallback(JSContext *cx) return JS_TRUE; } - if (LL_IS_ZERO(ctx->mOperationCallbackTime)) { - // Initialize mOperationCallbackTime to start timing how long the + PRUint32 callbackCount = ++ctx->mBranchCallbackCount; + + if (callbackCount & INITIALIZE_TIME_BRANCH_COUNT_MASK) { + return JS_TRUE; + } + + if (callbackCount == INITIALIZE_TIME_BRANCH_COUNT_MASK + 1 && + LL_IS_ZERO(ctx->mBranchCallbackTime)) { + // Initialize mBranchCallbackTime to start timing how long the // script has run - ctx->mOperationCallbackTime = PR_Now(); + ctx->mBranchCallbackTime = PR_Now(); ctx->mIsTrackingChromeCodeTime = ::JS_IsSystemObject(cx, ::JS_GetGlobalObject(cx)); - NS_ASSERTION(::JS_GetOperationLimit(cx) == INITIALIZE_TIME_OPERATION_LIMIT, - "The operation limit must match the initialization value"); - ::JS_SetOperationLimit(cx, MAYBE_GC_OPERATION_LIMIT); return JS_TRUE; } - NS_ASSERTION(::JS_GetOperationLimit(cx) == MAYBE_GC_OPERATION_LIMIT, - "The operation limit must match the long-running value"); + if (callbackCount & MAYBE_GC_BRANCH_COUNT_MASK) { + return JS_TRUE; + } - // XXX Save the operation callback time so we can restore it after the GC, + // XXX Save the branch callback time so we can restore it after the GC, // because GCing can cause JS to run on our context, causing our - // ScriptEvaluated to be called, and clearing our operation callback time. - // See bug 302333. - PRTime callbackTime = ctx->mOperationCallbackTime; + // ScriptEvaluated to be called, and clearing our branch callback time and + // count. See bug 302333. + PRTime callbackTime = ctx->mBranchCallbackTime; // Run the GC if we get this far. - ::JS_MaybeGC(cx); + JS_MaybeGC(cx); - // Now restore the callback time, in case they got reset. - ctx->mOperationCallbackTime = callbackTime; + // Now restore the callback time and count, in case they got reset. + ctx->mBranchCallbackTime = callbackTime; + ctx->mBranchCallbackCount = callbackCount; PRTime now = PR_Now(); @@ -863,9 +871,7 @@ nsJSContext::DOMOperationCallback(JSContext *cx) nsresult rv; // Check if we should offer the option to debug - JSStackFrame* fp = ::JS_GetScriptedCaller(cx, NULL); - PRBool debugPossible = (fp != nsnull && - cx->debugHooks->debuggerHandler != nsnull); + PRBool debugPossible = (cx->debugHooks->debuggerHandler != nsnull); #ifdef MOZ_JSDEBUGGER // Get the debugger service if necessary. if (debugPossible) { @@ -933,7 +939,6 @@ nsJSContext::DOMOperationCallback(JSContext *cx) } // Append file and line number information, if available - JSScript *script = fp ? ::JS_GetFrameScript(cx, fp) : nsnull; if (script) { const char *filename = ::JS_GetScriptFilename(cx, script); if (filename) { @@ -989,18 +994,17 @@ nsJSContext::DOMOperationCallback(JSContext *cx) } } - ctx->mOperationCallbackTime = PR_Now(); + ctx->mBranchCallbackTime = PR_Now(); return JS_TRUE; } else if ((buttonPressed == 2) && debugPossible) { // Debug the script jsval rval; - switch(cx->debugHooks->debuggerHandler(cx, script, ::JS_GetFramePC(cx, fp), - &rval, + switch(cx->debugHooks->debuggerHandler(cx, script, cx->fp->pc, &rval, cx->debugHooks-> debuggerHandlerData)) { case JSTRAP_RETURN: - fp->rval = rval; + cx->fp->rval = rval; return JS_TRUE; case JSTRAP_ERROR: cx->throwing = JS_FALSE; @@ -1088,7 +1092,9 @@ nsJSContext::nsJSContext(JSRuntime *aRuntime) : mGCOnDestruction(PR_TRUE) ++sContextCount; - mDefaultJSOptions = JSOPTION_PRIVATE_IS_NSISUPPORTS | JSOPTION_ANONFUNFIX; + mDefaultJSOptions = JSOPTION_PRIVATE_IS_NSISUPPORTS + | JSOPTION_NATIVE_BRANCH_CALLBACK + | JSOPTION_ANONFUNFIX; // Let xpconnect resync its JSContext tracker. We do this before creating // a new JSContext just in case the heap manager recycles the JSContext @@ -1107,8 +1113,7 @@ nsJSContext::nsJSContext(JSRuntime *aRuntime) : mGCOnDestruction(PR_TRUE) JSOptionChangedCallback, this); - ::JS_SetOperationCallback(mContext, DOMOperationCallback, - INITIALIZE_TIME_OPERATION_LIMIT); + ::JS_SetBranchCallback(mContext, DOMBranchCallback); static JSLocaleCallbacks localeCallbacks = { @@ -1124,7 +1129,8 @@ nsJSContext::nsJSContext(JSRuntime *aRuntime) : mGCOnDestruction(PR_TRUE) mNumEvaluations = 0; mTerminations = nsnull; mScriptsEnabled = PR_TRUE; - mOperationCallbackTime = LL_ZERO; + mBranchCallbackCount = 0; + mBranchCallbackTime = LL_ZERO; mProcessingScriptTag = PR_FALSE; mIsTrackingChromeCodeTime = PR_FALSE; } @@ -1161,8 +1167,8 @@ nsJSContext::Unlink() // Clear our entry in the JSContext, bugzilla bug 66413 ::JS_SetContextPrivate(mContext, nsnull); - // Clear the operation callback, bugzilla bug 238218 - ::JS_ClearOperationCallback(mContext); + // Clear the branch callback, bugzilla bug 238218 + ::JS_SetBranchCallback(mContext, nsnull); // Unregister our "javascript.options.*" pref-changed callback. nsContentUtils::UnregisterPrefCallback(js_options_dot_str, @@ -1469,12 +1475,10 @@ nsJSContext::EvaluateString(const nsAString& aScript, return NS_ERROR_FAILURE; } - // The result of evaluation, used only if there were no errors. This need - // not be a GC root currently, provided we run the GC only from the - // operation callback or from ScriptEvaluated. - // - // TODO: use JS_Begin/EndRequest to keep the GC from racing with JS - // execution on any thread. + // The result of evaluation, used only if there were no errors. This need + // not be a GC root currently, provided we run the GC only from the branch + // callback or from ScriptEvaluated. TODO: use JS_Begin/EndRequest to keep + // the GC from racing with JS execution on any thread. jsval val; nsJSContext::TerminationFuncHolder holder(this); @@ -1679,9 +1683,10 @@ nsJSContext::ExecuteScript(void *aScriptObject, return NS_ERROR_FAILURE; } - // The result of evaluation, used only if there were no errors. This need - // not be a GC root currently, provided we run the GC only from the - // operation callback or from ScriptEvaluated. + // The result of evaluation, used only if there were no errors. This need + // not be a GC root currently, provided we run the GC only from the branch + // callback or from ScriptEvaluated. TODO: use JS_Begin/EndRequest to keep + // the GC from racing with JS execution on any thread. jsval val; JSBool ok; @@ -3220,8 +3225,8 @@ nsJSContext::ScriptEvaluated(PRBool aTerminated) } #endif - mOperationCallbackTime = LL_ZERO; - ::JS_SetOperationLimit(mContext, INITIALIZE_TIME_OPERATION_LIMIT); + mBranchCallbackCount = 0; + mBranchCallbackTime = LL_ZERO; } nsresult diff --git a/dom/src/base/nsJSEnvironment.h b/dom/src/base/nsJSEnvironment.h index dd681d790b8..eb0ff5fa23e 100644 --- a/dom/src/base/nsJSEnvironment.h +++ b/dom/src/base/nsJSEnvironment.h @@ -279,7 +279,8 @@ private: PRPackedBool mProcessingScriptTag; PRPackedBool mIsTrackingChromeCodeTime; - PRTime mOperationCallbackTime; + PRUint32 mBranchCallbackCount; + PRTime mBranchCallbackTime; PRUint32 mDefaultJSOptions; // mGlobalWrapperRef is used only to hold a strong reference to the @@ -291,7 +292,8 @@ private: static int PR_CALLBACK JSOptionChangedCallback(const char *pref, void *data); - static JSBool JS_DLL_CALLBACK DOMOperationCallback(JSContext *cx); + static JSBool JS_DLL_CALLBACK DOMBranchCallback(JSContext *cx, + JSScript *script); }; class nsIJSRuntimeService; diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 2376b4be518..33e6d6b37c1 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -2105,19 +2105,14 @@ JS_CallFunctionValue(JSContext *cx, JSObject *obj, jsval fval, uintN argc, */ #define JS_MAX_OPERATION_LIMIT ((uint32) 0x7FFFFFFF) -#define JS_OPERATION_WEIGHT_BASE 4096 - /* * Set the operation callback that the engine calls periodically after * the internal operation count reaches the specified limit. * - * When operationLimit is JS_OPERATION_WEIGHT_BASE, the callback will be - * called at least after each backward jump in the interpreter. To minimize - * the overhead of the callback invocation we suggest at least - * - * 100 * JS_OPERATION_WEIGHT_BASE - * - * as a value for operationLimit. + * When operationLimit is 4096, the callback will be called at least after + * each backward jump in the interpreter. To minimize the overhead of the + * callback invocation we suggest at least 100*4096 as a value for + * operationLimit. */ extern JS_PUBLIC_API(void) JS_SetOperationCallback(JSContext *cx, JSOperationCallback callback, diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index afd13beaccc..dc59b0dbc83 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -1075,8 +1075,8 @@ extern JSErrorFormatString js_ErrorFormatString[JSErr_Limit]; #define JSOW_SET_PROPERTY 20 #define JSOW_NEW_PROPERTY 200 #define JSOW_DELETE_PROPERTY 30 -#define JSOW_ENTER_SHARP JS_OPERATION_WEIGHT_BASE -#define JSOW_SCRIPT_JUMP JS_OPERATION_WEIGHT_BASE +#define JSOW_ENTER_SHARP 4096 +#define JSOW_SCRIPT_JUMP 4096 /* * Reset the operation count and call the operation callback assuming that the diff --git a/js/src/xpconnect/src/xpccomponents.cpp b/js/src/xpconnect/src/xpccomponents.cpp index 1dfcd4b83fe..332cd356f70 100644 --- a/js/src/xpconnect/src/xpccomponents.cpp +++ b/js/src/xpconnect/src/xpccomponents.cpp @@ -3354,9 +3354,11 @@ public: NS_DECL_ISUPPORTS private: - static JSBool JS_DLL_CALLBACK ContextHolderOperationCallback(JSContext *cx); + static JSBool JS_DLL_CALLBACK ContextHolderBranchCallback(JSContext *cx, + JSScript *script); XPCAutoJSContext mJSContext; + JSBranchCallback mOrigBranchCallback; JSContext* mOrigCx; }; @@ -3364,49 +3366,38 @@ NS_IMPL_ISUPPORTS0(ContextHolder) ContextHolder::ContextHolder(JSContext *aOuterCx, JSObject *aSandbox) : mJSContext(JS_NewContext(JS_GetRuntime(aOuterCx), 1024), JS_FALSE), + mOrigBranchCallback(nsnull), mOrigCx(aOuterCx) { - if(mJSContext) - { + if (mJSContext) { JS_SetOptions(mJSContext, JSOPTION_DONT_REPORT_UNCAUGHT | JSOPTION_PRIVATE_IS_NSISUPPORTS); JS_SetGlobalObject(mJSContext, aSandbox); JS_SetContextPrivate(mJSContext, this); - if(JS_GetOperationCallback(aOuterCx)) - { - JS_SetOperationCallback(mJSContext, ContextHolderOperationCallback, - JS_GetOperationLimit(aOuterCx)); + // Now cache the original branch callback + mOrigBranchCallback = JS_SetBranchCallback(aOuterCx, nsnull); + JS_SetBranchCallback(aOuterCx, mOrigBranchCallback); + + if (mOrigBranchCallback) { + JS_SetBranchCallback(mJSContext, ContextHolderBranchCallback); } } } JSBool JS_DLL_CALLBACK -ContextHolder::ContextHolderOperationCallback(JSContext *cx) +ContextHolder::ContextHolderBranchCallback(JSContext *cx, JSScript *script) { ContextHolder* thisObject = static_cast(JS_GetContextPrivate(cx)); NS_ASSERTION(thisObject, "How did that happen?"); - JSContext *origCx = thisObject->mOrigCx; - JSOperationCallback callback = JS_GetOperationCallback(origCx); - JSBool ok = JS_TRUE; - if(callback) - { - ok = callback(origCx); - callback = JS_GetOperationCallback(origCx); - if(callback) - { - // If the callback is still set in the original context, reflect - // a possibly updated operation limit into cx. - JS_SetOperationLimit(cx, JS_GetOperationLimit(origCx)); - return ok; - } + if (thisObject->mOrigBranchCallback) { + return (thisObject->mOrigBranchCallback)(thisObject->mOrigCx, script); } - JS_ClearOperationCallback(cx); - return ok; + return JS_TRUE; } /***************************************************************************/