Bug 916949 - Minor refactor and clean-up of property access logic in VM. r=jorendorff
authorKannan Vijayan <kvijayan@mozilla.com>
Tue, 24 Sep 2013 14:41:14 -0400
changeset 162290 c5550f96b7c1e99a54b91732f53d96d458ff7ede
parent 162289 482d31cbd7b83a14bee4a2ecdeb4919d6b6b9753
child 162291 35295ad5d1e82cd67781a5b509c2515e3a430a4e
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs916949
milestone27.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 916949 - Minor refactor and clean-up of property access logic in VM. r=jorendorff
js/src/jit/BaselineIC.cpp
js/src/jit/IonCaches.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/vm/Debugger.cpp
js/src/vm/Interpreter-inl.h
js/src/vm/Interpreter.cpp
--- a/js/src/jit/BaselineIC.cpp
+++ b/js/src/jit/BaselineIC.cpp
@@ -5960,23 +5960,18 @@ DoGetPropFallback(JSContext *cx, Baselin
             return true;
         }
     }
 
     RootedObject obj(cx, ToObjectFromStack(cx, val));
     if (!obj)
         return false;
 
-    if (obj->getOps()->getProperty) {
-        if (!JSObject::getGeneric(cx, obj, obj, id, res))
-            return false;
-    } else {
-        if (!GetPropertyHelper(cx, obj, id, 0, res))
-            return false;
-    }
+    if (!JSObject::getGeneric(cx, obj, obj, id, res))
+        return false;
 
 #if JS_HAS_NO_SUCH_METHOD
     // Handle objects with __noSuchMethod__.
     if (op == JSOP_CALLPROP && JS_UNLIKELY(res.isPrimitive()) && val.isObject()) {
         if (!OnUnknownMethod(cx, obj, IdToValue(id), res))
             return false;
     }
 #endif
--- a/js/src/jit/IonCaches.cpp
+++ b/js/src/jit/IonCaches.cpp
@@ -1726,23 +1726,18 @@ GetPropertyIC::update(JSContext *cx, siz
         // Do not re-invalidate if the lookup already caused invalidation.
         if (!topScript->hasIonScript())
             return true;
 
         return Invalidate(cx, topScript);
     }
 
     RootedId id(cx, NameToId(name));
-    if (obj->getOps()->getProperty) {
-        if (!JSObject::getGeneric(cx, obj, obj, id, vp))
-            return false;
-    } else {
-        if (!GetPropertyHelper(cx, obj, id, 0, vp))
-            return false;
-    }
+    if (!JSObject::getGeneric(cx, obj, obj, id, vp))
+        return false;
 
     if (!cache.idempotent()) {
         RootedScript script(cx);
         jsbytecode *pc;
         cache.getScriptedLocation(&script, &pc);
 
         // If the cache is idempotent, the property exists so we don't have to
         // call __noSuchMethod__.
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -665,17 +665,17 @@ DefinePropertyOnObject(JSContext *cx, Ha
                 if (!shape->configurable() &&
                     (!shape->hasDefaultGetter() || !shape->hasDefaultSetter()) &&
                     desc.isDataDescriptor() &&
                     (desc.hasWritable() ? desc.writable() : shape->writable()))
                 {
                     return Reject(cx, JSMSG_CANT_REDEFINE_PROP, throwError, id, rval);
                 }
 
-                if (!js_NativeGet(cx, obj, obj2, shape, 0, &v))
+                if (!js_NativeGet(cx, obj, obj2, shape, &v))
                     return false;
             }
 
             if (desc.isDataDescriptor()) {
                 if (!shapeDataDescriptor)
                     break;
 
                 bool same;
@@ -4016,17 +4016,16 @@ js::HasOwnProperty<NoGC>(JSContext *cx, 
 
 template <AllowGC allowGC>
 static JS_ALWAYS_INLINE bool
 NativeGetInline(JSContext *cx,
                 typename MaybeRooted<JSObject*, allowGC>::HandleType obj,
                 typename MaybeRooted<JSObject*, allowGC>::HandleType receiver,
                 typename MaybeRooted<JSObject*, allowGC>::HandleType pobj,
                 typename MaybeRooted<Shape*, allowGC>::HandleType shape,
-                unsigned getHow,
                 typename MaybeRooted<Value, allowGC>::MutableHandleType vp)
 {
     JS_ASSERT(pobj->isNative());
 
     if (shape->hasSlot()) {
         vp.set(pobj->nativeGetSlot(shape->slot()));
         JS_ASSERT(!vp.isMagic());
         JS_ASSERT_IF(!pobj->hasSingletonType() && shape->hasDefaultGetter(),
@@ -4071,19 +4070,19 @@ NativeGetInline(JSContext *cx,
     if (shape->hasSlot() && pobj->nativeContains(cx, shape))
         pobj->nativeSetSlot(shape->slot(), vp);
 
     return true;
 }
 
 bool
 js_NativeGet(JSContext *cx, Handle<JSObject*> obj, Handle<JSObject*> pobj, Handle<Shape*> shape,
-             unsigned getHow, MutableHandle<Value> vp)
+             MutableHandle<Value> vp)
 {
-    return NativeGetInline<CanGC>(cx, obj, obj, pobj, shape, getHow, vp);
+    return NativeGetInline<CanGC>(cx, obj, obj, pobj, shape, vp);
 }
 
 
 bool
 js_NativeSet(JSContext *cx, Handle<JSObject*> obj, Handle<JSObject*> receiver,
              HandleShape shape, bool strict, MutableHandleValue vp)
 {
     JS_ASSERT(obj->isNative());
@@ -4130,17 +4129,16 @@ js_NativeSet(JSContext *cx, Handle<JSObj
 }
 
 template <AllowGC allowGC>
 static JS_ALWAYS_INLINE bool
 GetPropertyHelperInline(JSContext *cx,
                         typename MaybeRooted<JSObject*, allowGC>::HandleType obj,
                         typename MaybeRooted<JSObject*, allowGC>::HandleType receiver,
                         typename MaybeRooted<jsid, allowGC>::HandleType id,
-                        uint32_t getHow,
                         typename MaybeRooted<Value, allowGC>::MutableHandleType vp)
 {
     /* This call site is hot -- use the always-inlined variant of LookupPropertyWithFlags(). */
     typename MaybeRooted<JSObject*, allowGC>::RootType obj2(cx);
     typename MaybeRooted<Shape*, allowGC>::RootType shape(cx);
     if (!LookupPropertyWithFlagsInline<allowGC>(cx, obj, id, cx->resolveFlags, &obj2, &shape))
         return false;
 
@@ -4228,40 +4226,34 @@ GetPropertyHelperInline(JSContext *cx,
     }
 
     if (IsImplicitDenseElement(shape)) {
         vp.set(obj2->getDenseElement(JSID_TO_INT(id)));
         return true;
     }
 
     /* This call site is hot -- use the always-inlined variant of js_NativeGet(). */
-    if (!NativeGetInline<allowGC>(cx, obj, receiver, obj2, shape, getHow, vp))
+    if (!NativeGetInline<allowGC>(cx, obj, receiver, obj2, shape, vp))
         return false;
 
     return true;
 }
 
 bool
-js::GetPropertyHelper(JSContext *cx, HandleObject obj, HandleId id, uint32_t getHow, MutableHandleValue vp)
-{
-    return GetPropertyHelperInline<CanGC>(cx, obj, obj, id, getHow, vp);
-}
-
-bool
 baseops::GetProperty(JSContext *cx, HandleObject obj, HandleObject receiver, HandleId id, MutableHandleValue vp)
 {
     /* This call site is hot -- use the always-inlined variant of GetPropertyHelper(). */
-    return GetPropertyHelperInline<CanGC>(cx, obj, receiver, id, 0, vp);
+    return GetPropertyHelperInline<CanGC>(cx, obj, receiver, id, vp);
 }
 
 bool
 baseops::GetPropertyNoGC(JSContext *cx, JSObject *obj, JSObject *receiver, jsid id, Value *vp)
 {
     AutoAssertNoException nogc(cx);
-    return GetPropertyHelperInline<NoGC>(cx, obj, receiver, id, 0, vp);
+    return GetPropertyHelperInline<NoGC>(cx, obj, receiver, id, vp);
 }
 
 static JS_ALWAYS_INLINE bool
 LookupPropertyPureInline(JSObject *obj, jsid id, JSObject **objp, Shape **propp)
 {
     if (!obj->isNative())
         return false;
 
@@ -4436,17 +4428,17 @@ bool
 baseops::GetElement(JSContext *cx, HandleObject obj, HandleObject receiver, uint32_t index,
                     MutableHandleValue vp)
 {
     RootedId id(cx);
     if (!IndexToId(cx, index, &id))
         return false;
 
     /* This call site is hot -- use the always-inlined variant of js_GetPropertyHelper(). */
-    return GetPropertyHelperInline<CanGC>(cx, obj, receiver, id, 0, vp);
+    return GetPropertyHelperInline<CanGC>(cx, obj, receiver, id, vp);
 }
 
 static bool
 MaybeReportUndeclaredVarAssignment(JSContext *cx, JSString *propname)
 {
     {
         JSScript *script = cx->currentScript(NULL, JSContext::ALLOW_CROSS_COMPARTMENT);
         if (!script)
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -904,16 +904,17 @@ class JSObject : public js::ObjectImpl
                               js::SpecialId sid, js::HandleValue value,
                               JSPropertyOp getter = JS_PropertyStub,
                               JSStrictPropertyOp setter = JS_StrictPropertyStub,
                               unsigned attrs = JSPROP_ENUMERATE);
 
     static bool getGeneric(JSContext *cx, js::HandleObject obj, js::HandleObject receiver,
                            js::HandleId id, js::MutableHandleValue vp)
     {
+        JS_ASSERT(!!obj->getOps()->getGeneric == !!obj->getOps()->getProperty);
         js::GenericIdOp op = obj->getOps()->getGeneric;
         if (op) {
             if (!op(cx, obj, receiver, id, vp))
                 return false;
         } else {
             if (!js::baseops::GetProperty(cx, obj, receiver, id, vp))
                 return false;
         }
@@ -1392,43 +1393,27 @@ extern bool
 LookupNameWithGlobalDefault(JSContext *cx, HandlePropertyName name, HandleObject scopeChain,
                             MutableHandleObject objp);
 
 }
 
 extern JSObject *
 js_FindVariableScope(JSContext *cx, JSFunction **funp);
 
-/*
- * NB: js_NativeGet and js_NativeSet are called with the scope containing shape
- * (pobj's scope for Get, obj's for Set) locked, and on successful return, that
- * scope is again locked.  But on failure, both functions return false with the
- * scope containing shape unlocked.
- */
 extern bool
 js_NativeGet(JSContext *cx, js::Handle<JSObject*> obj, js::Handle<JSObject*> pobj,
-             js::Handle<js::Shape*> shape, unsigned getHow, js::MutableHandle<js::Value> vp);
+             js::Handle<js::Shape*> shape, js::MutableHandle<js::Value> vp);
 
 extern bool
 js_NativeSet(JSContext *cx, js::Handle<JSObject*> obj, js::Handle<JSObject*> receiver,
              js::Handle<js::Shape*> shape, bool strict, js::MutableHandleValue vp);
 
 namespace js {
 
 bool
-GetPropertyHelper(JSContext *cx, HandleObject obj, HandleId id, uint32_t getHow, MutableHandleValue vp);
-
-inline bool
-GetPropertyHelper(JSContext *cx, HandleObject obj, PropertyName *name, uint32_t getHow, MutableHandleValue vp)
-{
-    RootedId id(cx, NameToId(name));
-    return GetPropertyHelper(cx, obj, id, getHow, vp);
-}
-
-bool
 LookupPropertyPure(JSObject *obj, jsid id, JSObject **objp, Shape **propp);
 
 bool
 GetPropertyPure(ThreadSafeContext *cx, JSObject *obj, jsid id, Value *vp);
 
 inline bool
 GetPropertyPure(ThreadSafeContext *cx, JSObject *obj, PropertyName *name, Value *vp)
 {
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -897,17 +897,17 @@ Debugger::parseResumptionValue(Maybe<Aut
              shape->isDataDescriptor();
     }
     if (!okResumption) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_DEBUG_BAD_RESUMPTION);
         return handleUncaughtException(ac, vp, callHook);
     }
 
     RootedValue v(cx, vp.get());
-    if (!js_NativeGet(cx, obj, obj, shape, 0, &v) || !unwrapDebuggeeValue(cx, &v))
+    if (!js_NativeGet(cx, obj, obj, shape, &v) || !unwrapDebuggeeValue(cx, &v))
         return handleUncaughtException(ac, &v, callHook);
 
     ac.destroy();
     if (!cx->compartment()->wrap(cx, &v)) {
         vp.setUndefined();
         return JSTRAP_ERROR;
     }
     vp.set(v);
--- a/js/src/vm/Interpreter-inl.h
+++ b/js/src/vm/Interpreter-inl.h
@@ -125,27 +125,27 @@ ValuePropertyBearer(JSContext *cx, Stack
 
     JS_ASSERT(v.isNull() || v.isUndefined());
     js_ReportIsNullOrUndefined(cx, spindex, v, NullPtr());
     return NULL;
 }
 
 inline bool
 NativeGet(JSContext *cx, JSObject *objArg, JSObject *pobjArg, Shape *shapeArg,
-          unsigned getHow, MutableHandleValue vp)
+          MutableHandleValue vp)
 {
     if (shapeArg->isDataDescriptor() && shapeArg->hasDefaultGetter()) {
         /* Fast path for Object instance properties. */
         JS_ASSERT(shapeArg->hasSlot());
         vp.set(pobjArg->nativeGetSlot(shapeArg->slot()));
     } else {
         RootedObject obj(cx, objArg);
         RootedObject pobj(cx, pobjArg);
         RootedShape shape(cx, shapeArg);
-        if (!js_NativeGet(cx, obj, pobj, shape, getHow, vp))
+        if (!js_NativeGet(cx, obj, pobj, shape, vp))
             return false;
     }
     return true;
 }
 
 inline bool
 GetLengthProperty(const Value &lval, MutableHandleValue vp)
 {
@@ -199,17 +199,17 @@ FetchName(JSContext *cx, HandleObject ob
     if (!obj->isNative() || !obj2->isNative()) {
         Rooted<jsid> id(cx, NameToId(name));
         if (!JSObject::getGeneric(cx, obj, obj, id, vp))
             return false;
     } else {
         Rooted<JSObject*> normalized(cx, obj);
         if (normalized->getClass() == &WithObject::class_ && !shape->hasDefaultGetter())
             normalized = &normalized->as<WithObject>().object();
-        if (!NativeGet(cx, normalized, obj2, shape, 0, vp))
+        if (!NativeGet(cx, normalized, obj2, shape, vp))
             return false;
     }
     return true;
 }
 
 inline bool
 FetchNameNoGC(JSObject *pobj, Shape *shape, MutableHandleValue vp)
 {
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -258,23 +258,18 @@ GetPropertyOperation(JSContext *cx, Stac
     if (!obj) {
         obj = ToObjectFromStack(cx, lval);
         if (!obj)
             return false;
     }
 
     bool wasObject = lval.isObject();
 
-    if (obj->getOps()->getProperty) {
-        if (!JSObject::getGeneric(cx, obj, obj, id, vp))
-            return false;
-    } else {
-        if (!GetPropertyHelper(cx, obj, id, 0, vp))
-            return false;
-    }
+    if (!JSObject::getGeneric(cx, obj, obj, id, vp))
+        return false;
 
 #if JS_HAS_NO_SUCH_METHOD
     if (op == JSOP_CALLPROP &&
         JS_UNLIKELY(vp.isPrimitive()) &&
         wasObject)
     {
         if (!OnUnknownMethod(cx, obj, IdToValue(id), vp))
             return false;