From 0a664647e1deb9b2035e1590368501a159fd19aa Mon Sep 17 00:00:00 2001 From: "bzbarsky%mit.edu" Date: Sun, 24 Sep 2006 16:44:51 +0000 Subject: [PATCH] Don't copy the string data from mJSVal, since we're just going to keep the JSString alive. Bug 311582, r=jst, sr=brendan git-svn-id: svn://10.0.0.236/trunk@212249 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/js/src/xpconnect/src/xpcprivate.h | 8 +++- mozilla/js/src/xpconnect/src/xpcvariant.cpp | 47 ++++++++++++++------- 2 files changed, 39 insertions(+), 16 deletions(-) diff --git a/mozilla/js/src/xpconnect/src/xpcprivate.h b/mozilla/js/src/xpconnect/src/xpcprivate.h index 3dddfa664dd..f9fa7cef8d8 100644 --- a/mozilla/js/src/xpconnect/src/xpcprivate.h +++ b/mozilla/js/src/xpconnect/src/xpcprivate.h @@ -3540,6 +3540,9 @@ class XPCVariant : public nsIVariant public: NS_DECL_ISUPPORTS NS_DECL_NSIVARIANT + // If this class ever implements nsIWritableVariant, take special care with + // the case when mJSVal is JSVAL_STRING, since we don't own the data in + // that case. // We #define and iid so that out module local code can use QI to detect // if a given nsIVariant is in fact an XPCVariant. @@ -3549,7 +3552,7 @@ public: jsval GetJSVal() const {return mJSVal;} - XPCVariant(); + XPCVariant(JSRuntime* aJSRuntime); /** * Convert a variant into a jsval. @@ -3574,6 +3577,9 @@ protected: protected: nsDiscriminatedUnion mData; jsval mJSVal; + + // For faster GC-thing locking and unlocking + JSRuntime* mJSRuntime; }; NS_DEFINE_STATIC_IID_ACCESSOR(XPCVariant, XPCVARIANT_IID) diff --git a/mozilla/js/src/xpconnect/src/xpcvariant.cpp b/mozilla/js/src/xpconnect/src/xpcvariant.cpp index 6229445edd2..dce5c245379 100644 --- a/mozilla/js/src/xpconnect/src/xpcvariant.cpp +++ b/mozilla/js/src/xpconnect/src/xpcvariant.cpp @@ -44,30 +44,31 @@ NS_IMPL_ISUPPORTS2_CI(XPCVariant, XPCVariant, nsIVariant) -XPCVariant::XPCVariant() - : mJSVal(JSVAL_VOID) +XPCVariant::XPCVariant(JSRuntime* aJSRuntime) + : mJSVal(JSVAL_VOID), + mJSRuntime(aJSRuntime) { nsVariant::Initialize(&mData); } XPCVariant::~XPCVariant() { - nsVariant::Cleanup(&mData); + // If mJSVal is JSVAL_STRING, we don't need to clean anything up; + // simply unlocking the string is good. + if(!JSVAL_IS_STRING(mJSVal)) + nsVariant::Cleanup(&mData); if(JSVAL_IS_GCTHING(mJSVal)) { - JSRuntime* rt; - nsIJSRuntimeService* rtsrvc = nsXPConnect::GetJSRuntimeService(); - - if(rtsrvc && NS_SUCCEEDED(rtsrvc->GetRuntime(&rt))) - JS_RemoveRootRT(rt, &mJSVal); + NS_ASSERTION(mJSRuntime, "Must have a runtime!"); + JS_UnlockGCThingRT(mJSRuntime, JSVAL_TO_GCTHING(mJSVal)); } } // static XPCVariant* XPCVariant::newVariant(XPCCallContext& ccx, jsval aJSVal) { - XPCVariant* variant = new XPCVariant(); + XPCVariant* variant = new XPCVariant(JS_GetRuntime(ccx)); if(!variant) return nsnull; @@ -77,9 +78,9 @@ XPCVariant* XPCVariant::newVariant(XPCCallContext& ccx, jsval aJSVal) if(JSVAL_IS_GCTHING(variant->mJSVal)) { - JSRuntime* rt; - if(NS_FAILED(ccx.GetRuntime()->GetJSRuntimeService()->GetRuntime(&rt))|| - !JS_AddNamedRootRT(rt, &variant->mJSVal, "XPCVariant::mJSVal")) + // use JS_LockGCThingRT, because we get better performance in a lot of + // cases than with adding a named GC root. + if(!JS_LockGCThing(ccx, JSVAL_TO_GCTHING(variant->mJSVal))) { NS_RELEASE(variant); // Also sets variant to nsnull. } @@ -254,9 +255,25 @@ JSBool XPCVariant::InitializeData(XPCCallContext& ccx) return NS_SUCCEEDED(nsVariant::SetToEmpty(&mData)); if(JSVAL_IS_STRING(mJSVal)) { - return NS_SUCCEEDED(nsVariant::SetFromWStringWithSize(&mData, - (PRUint32)JS_GetStringLength(JSVAL_TO_STRING(mJSVal)), - (PRUnichar*)JS_GetStringChars(JSVAL_TO_STRING(mJSVal)))); + // Make our string immutable. This will also ensure null-termination, + // which nsVariant assumes for its PRUnichar* stuff. + JSString* str = JSVAL_TO_STRING(mJSVal); + if(!JS_MakeStringImmutable(ccx, str)) + return JS_FALSE; + + // Don't use nsVariant::SetFromWStringWithSize, because that will copy + // the data. Just handle this ourselves. Note that it's ok to not + // copy because we added mJSVal as a GC root. + NS_ASSERTION(mData.mType == nsIDataType::VTYPE_EMPTY, + "Why do we already have data?"); + + mData.u.wstr.mWStringValue = + NS_REINTERPRET_CAST(PRUnichar*, JS_GetStringChars(str)); + mData.u.wstr.mWStringLength = + NS_REINTERPRET_CAST(PRUint32, JS_GetStringLength(str)); + mData.mType = nsIDataType::VTYPE_WSTRING_SIZE_IS; + + return JS_TRUE; } // leaving only JSObject...