From 88b1d4572f4e2354d780c733aa5f1aff8c362487 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Tue, 11 Nov 2014 11:56:44 +0100 Subject: [PATCH] Bug 1094255 - Optimize GetElements on NodeLists to make fun.apply(x, nodeList) faster. r=bz,evilpie --- dom/bindings/Codegen.py | 14 +-- js/public/Class.h | 42 ++++++- js/src/builtin/TypedObject.cpp | 2 +- .../tests/basic/function-apply-proxy.js | 26 +++++ js/src/jsarray.cpp | 105 +++++++++++++----- js/src/jsfriendapi.cpp | 8 -- js/src/jsfriendapi.h | 13 +-- js/src/jsproxy.h | 4 +- js/src/proxy/BaseProxyHandler.cpp | 6 +- js/src/proxy/Proxy.cpp | 14 +-- js/src/proxy/Proxy.h | 4 +- js/src/vm/ScopeObject.cpp | 4 +- js/xpconnect/src/XPCWrappedNativeJSOps.cpp | 2 +- js/xpconnect/src/xpcprivate.h | 4 +- 14 files changed, 175 insertions(+), 73 deletions(-) create mode 100644 js/src/jit-test/tests/basic/function-apply-proxy.js diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 7e9596bcfa37..b8b78b56fad1 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -409,7 +409,7 @@ class CGDOMJSClass(CGThing): nullptr, /* deleteGeneric */ nullptr, /* watch */ nullptr, /* unwatch */ - nullptr, /* slice */ + nullptr, /* getElements */ nullptr, /* enumerate */ JS_ObjectToOuterObject /* thisObject */ } @@ -10716,7 +10716,7 @@ class CGDOMJSProxyHandler_finalize(ClassMethod): finalizeHook(self.descriptor, FINALIZE_HOOK_NAME, self.args[0].name).define()) -class CGDOMJSProxyHandler_slice(ClassMethod): +class CGDOMJSProxyHandler_getElements(ClassMethod): def __init__(self, descriptor): assert descriptor.supportsIndexedProperties() @@ -10724,8 +10724,8 @@ class CGDOMJSProxyHandler_slice(ClassMethod): Argument('JS::Handle', 'proxy'), Argument('uint32_t', 'begin'), Argument('uint32_t', 'end'), - Argument('JS::Handle', 'array')] - ClassMethod.__init__(self, "slice", "bool", args, virtual=True, override=True, const=True) + Argument('js::ElementAdder*', 'adder')] + ClassMethod.__init__(self, "getElements", "bool", args, virtual=True, override=True, const=True) self.descriptor = descriptor def getBody(self): @@ -10738,7 +10738,7 @@ class CGDOMJSProxyHandler_slice(ClassMethod): 'jsvalRef': 'temp', 'jsvalHandle': '&temp', 'obj': 'proxy', - 'successCode': ("js::UnsafeDefineElement(cx, array, index - begin, temp);\n" + 'successCode': ("adder->append(cx, temp);\n" "continue;\n") } get = CGProxyIndexedGetter(self.descriptor, templateValues, False, False).define() @@ -10763,7 +10763,7 @@ class CGDOMJSProxyHandler_slice(ClassMethod): if (!js::GetObjectProto(cx, proxy, &proto)) { return false; } - return js::SliceSlowly(cx, proto, proxy, ourEnd, end, array); + return js::GetElementsWithAdder(cx, proto, proxy, ourEnd, end, adder); } return true; @@ -10836,7 +10836,7 @@ class CGDOMJSProxyHandler(CGClass): if descriptor.supportsIndexedProperties(): - methods.append(CGDOMJSProxyHandler_slice(descriptor)) + methods.append(CGDOMJSProxyHandler_getElements(descriptor)) if (descriptor.operations['IndexedSetter'] is not None or (descriptor.operations['NamedSetter'] is not None and descriptor.interface.getExtendedAttribute('OverrideBuiltins'))): diff --git a/js/public/Class.h b/js/public/Class.h index 0c7b5063d629..50f5dd91c0c6 100644 --- a/js/public/Class.h +++ b/js/public/Class.h @@ -9,6 +9,7 @@ #ifndef js_Class_h #define js_Class_h +#include "mozilla/DebugOnly.h" #include "mozilla/NullPtr.h" #include "jstypes.h" @@ -226,9 +227,44 @@ typedef bool typedef bool (* UnwatchOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id); +class JS_FRIEND_API(ElementAdder) +{ + public: + enum GetBehavior { + // Check if the element exists before performing the Get and preserve + // holes. + CheckHasElemPreserveHoles, + + // Perform a Get operation, like obj[index] in JS. + GetElement + }; + + private: + // Only one of these is used. + JS::RootedObject resObj_; + JS::Value *vp_; + + uint32_t index_; + mozilla::DebugOnly length_; + GetBehavior getBehavior_; + + public: + ElementAdder(JSContext *cx, JSObject *obj, uint32_t length, GetBehavior behavior) + : resObj_(cx, obj), vp_(nullptr), index_(0), length_(length), getBehavior_(behavior) + {} + ElementAdder(JSContext *cx, JS::Value *vp, uint32_t length, GetBehavior behavior) + : resObj_(cx), vp_(vp), index_(0), length_(length), getBehavior_(behavior) + {} + + GetBehavior getBehavior() const { return getBehavior_; } + + void append(JSContext *cx, JS::HandleValue v); + void appendHole(); +}; + typedef bool -(* SliceOp)(JSContext *cx, JS::HandleObject obj, uint32_t begin, uint32_t end, - JS::HandleObject result); // result is actually preallocted. +(* GetElementsOp)(JSContext *cx, JS::HandleObject obj, uint32_t begin, uint32_t end, + ElementAdder *adder); // A generic type for functions mapping an object to another object, or null // if an error or exception was thrown on cx. @@ -364,7 +400,7 @@ struct ObjectOps DeleteGenericOp deleteGeneric; WatchOp watch; UnwatchOp unwatch; - SliceOp slice; // Optimized slice, can be null. + GetElementsOp getElements; JSNewEnumerateOp enumerate; ObjectOp thisObject; }; diff --git a/js/src/builtin/TypedObject.cpp b/js/src/builtin/TypedObject.cpp index 5518697ba3a5..22baa228c083 100644 --- a/js/src/builtin/TypedObject.cpp +++ b/js/src/builtin/TypedObject.cpp @@ -2404,7 +2404,7 @@ LazyArrayBufferTable::sizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) TypedObject::obj_setGenericAttributes, \ TypedObject::obj_deleteGeneric, \ nullptr, nullptr, /* watch/unwatch */ \ - nullptr, /* slice */ \ + nullptr, /* getElements */ \ TypedObject::obj_enumerate, \ nullptr, /* thisObject */ \ } \ diff --git a/js/src/jit-test/tests/basic/function-apply-proxy.js b/js/src/jit-test/tests/basic/function-apply-proxy.js new file mode 100644 index 000000000000..9a00fdb3de7e --- /dev/null +++ b/js/src/jit-test/tests/basic/function-apply-proxy.js @@ -0,0 +1,26 @@ +// fun.apply(null, proxy) should not invoke the proxy's Has trap. + +var proxy = new Proxy({}, { + get: function (target, name, proxy) { + switch (name) { + case "length": + return 2; + case "0": + return 15; + case "1": + return undefined; + default: + assertEq(false, true); + } + }, + has: function (target, name) { + assertEq(false, true); + } +}); +function foo() { + assertEq(arguments.length, 2); + assertEq(arguments[0], 15); + assertEq(1 in arguments, true); + assertEq(arguments[1], undefined); +} +foo.apply(null, proxy); diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 46209b82bb4b..8fa6f8f034e6 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -237,12 +237,51 @@ GetElement(JSContext *cx, HandleObject obj, IndexType index, bool *hole, Mutable return GetElement(cx, obj, obj, index, hole, vp); } -static bool -GetElementsSlow(JSContext *cx, HandleObject aobj, uint32_t length, Value *vp) +void +ElementAdder::append(JSContext *cx, HandleValue v) { - for (uint32_t i = 0; i < length; i++) { - if (!JSObject::getElement(cx, aobj, aobj, i, MutableHandleValue::fromMarkedLocation(&vp[i]))) - return false; + MOZ_ASSERT(index_ < length_); + if (resObj_) + resObj_->as().setDenseElementWithType(cx, index_++, v); + else + vp_[index_++] = v; +} + +void +ElementAdder::appendHole() +{ + MOZ_ASSERT(getBehavior_ == ElementAdder::CheckHasElemPreserveHoles); + MOZ_ASSERT(index_ < length_); + if (resObj_) { + MOZ_ASSERT(resObj_->as().getDenseElement(index_).isMagic(JS_ELEMENTS_HOLE)); + index_++; + } else { + vp_[index_++].setMagic(JS_ELEMENTS_HOLE); + } +} + +bool +js::GetElementsWithAdder(JSContext *cx, HandleObject obj, HandleObject receiver, + uint32_t begin, uint32_t end, ElementAdder *adder) +{ + MOZ_ASSERT(begin <= end); + + RootedValue val(cx); + for (uint32_t i = begin; i < end; i++) { + if (adder->getBehavior() == ElementAdder::CheckHasElemPreserveHoles) { + bool hole; + if (!GetElement(cx, obj, receiver, i, &hole, &val)) + return false; + if (hole) { + adder->appendHole(); + continue; + } + } else { + MOZ_ASSERT(adder->getBehavior() == ElementAdder::GetElement); + if (!JSObject::getElement(cx, obj, receiver, i, &val)) + return false; + } + adder->append(cx, val); } return true; @@ -272,7 +311,17 @@ js::GetElements(JSContext *cx, HandleObject aobj, uint32_t length, Value *vp) } } - return GetElementsSlow(cx, aobj, length, vp); + if (js::GetElementsOp op = aobj->getOps()->getElements) { + ElementAdder adder(cx, vp, length, ElementAdder::GetElement); + return op(cx, aobj, 0, length, &adder); + } + + for (uint32_t i = 0; i < length; i++) { + if (!JSObject::getElement(cx, aobj, aobj, i, MutableHandleValue::fromMarkedLocation(&vp[i]))) + return false; + } + + return true; } /* @@ -2815,6 +2864,24 @@ GetIndexedPropertiesInRange(JSContext *cx, HandleObject obj, uint32_t begin, uin return true; } +static bool +SliceSlowly(JSContext* cx, HandleObject obj, HandleObject receiver, + uint32_t begin, uint32_t end, HandleObject result) +{ + RootedValue value(cx); + for (uint32_t slot = begin; slot < end; slot++) { + bool hole; + if (!CheckForInterrupt(cx) || + !GetElement(cx, obj, receiver, slot, &hole, &value)) + { + return false; + } + if (!hole && !JSObject::defineElement(cx, result, slot - begin, value)) + return false; + } + return true; +} + static bool SliceSparse(JSContext *cx, HandleObject obj, uint32_t begin, uint32_t end, HandleObject result) { @@ -2913,14 +2980,16 @@ js::array_slice(JSContext *cx, unsigned argc, Value *vp) return false; TryReuseArrayType(obj, narr); - if (js::SliceOp op = obj->getOps()->slice) { - // Ensure that we have dense elements, so that DOM can use js::UnsafeDefineElement. + if (js::GetElementsOp op = obj->getOps()->getElements) { + // Ensure that we have dense elements, so that ElementAdder::append can + // use setDenseElementWithType. NativeObject::EnsureDenseResult result = narr->ensureDenseElements(cx, 0, end - begin); if (result == NativeObject::ED_FAILED) return false; if (result == NativeObject::ED_OK) { - if (!op(cx, obj, begin, end, narr)) + ElementAdder adder(cx, narr, end - begin, ElementAdder::CheckHasElemPreserveHoles); + if (!op(cx, obj, begin, end, &adder)) return false; args.rval().setObject(*narr); @@ -2943,24 +3012,6 @@ js::array_slice(JSContext *cx, unsigned argc, Value *vp) return true; } -JS_FRIEND_API(bool) -js::SliceSlowly(JSContext* cx, HandleObject obj, HandleObject receiver, - uint32_t begin, uint32_t end, HandleObject result) -{ - RootedValue value(cx); - for (uint32_t slot = begin; slot < end; slot++) { - bool hole; - if (!CheckForInterrupt(cx) || - !GetElement(cx, obj, receiver, slot, &hole, &value)) - { - return false; - } - if (!hole && !JSObject::defineElement(cx, result, slot - begin, value)) - return false; - } - return true; -} - /* ES5 15.4.4.20. */ static bool array_filter(JSContext *cx, unsigned argc, Value *vp) diff --git a/js/src/jsfriendapi.cpp b/js/src/jsfriendapi.cpp index 08fe7d241e81..c89076c61e2d 100644 --- a/js/src/jsfriendapi.cpp +++ b/js/src/jsfriendapi.cpp @@ -1386,14 +1386,6 @@ js::GetObjectMetadata(JSObject *obj) return obj->getMetadata(); } -JS_FRIEND_API(void) -js::UnsafeDefineElement(JSContext *cx, JS::HandleObject obj, uint32_t index, JS::HandleValue value) -{ - MOZ_ASSERT(obj->isNative()); - MOZ_ASSERT(index < obj->as().getDenseInitializedLength()); - obj->as().setDenseElementWithType(cx, index, value); -} - JS_FRIEND_API(bool) js_DefineOwnProperty(JSContext *cx, JSObject *objArg, jsid idArg, JS::Handle descriptor, bool *bp) diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index d7514cb5f7fb..c3a39c033fc2 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -323,7 +323,7 @@ namespace js { js::proxy_SetGenericAttributes, \ js::proxy_DeleteGeneric, \ js::proxy_Watch, js::proxy_Unwatch, \ - js::proxy_Slice, \ + js::proxy_GetElements, \ nullptr, /* enumerate */ \ nullptr, /* thisObject */ \ } \ @@ -411,8 +411,8 @@ proxy_Watch(JSContext *cx, JS::HandleObject obj, JS::HandleId id, JS::HandleObje extern JS_FRIEND_API(bool) proxy_Unwatch(JSContext *cx, JS::HandleObject obj, JS::HandleId id); extern JS_FRIEND_API(bool) -proxy_Slice(JSContext *cx, JS::HandleObject proxy, uint32_t begin, uint32_t end, - JS::HandleObject result); +proxy_GetElements(JSContext *cx, JS::HandleObject proxy, uint32_t begin, uint32_t end, + ElementAdder *adder); /* * A class of objects that return source code on demand. @@ -2578,12 +2578,9 @@ SetObjectMetadata(JSContext *cx, JS::HandleObject obj, JS::HandleObject metadata JS_FRIEND_API(JSObject *) GetObjectMetadata(JSObject *obj); -JS_FRIEND_API(void) -UnsafeDefineElement(JSContext *cx, JS::HandleObject obj, uint32_t index, JS::HandleValue value); - JS_FRIEND_API(bool) -SliceSlowly(JSContext* cx, JS::HandleObject obj, JS::HandleObject receiver, - uint32_t begin, uint32_t end, JS::HandleObject result); +GetElementsWithAdder(JSContext *cx, JS::HandleObject obj, JS::HandleObject receiver, + uint32_t begin, uint32_t end, js::ElementAdder *adder); JS_FRIEND_API(bool) ForwardToNative(JSContext *cx, JSNative native, const JS::CallArgs &args); diff --git a/js/src/jsproxy.h b/js/src/jsproxy.h index c9221fd0d5fb..2d122385609d 100644 --- a/js/src/jsproxy.h +++ b/js/src/jsproxy.h @@ -332,8 +332,8 @@ class JS_FRIEND_API(BaseProxyHandler) JS::HandleObject callable) const; virtual bool unwatch(JSContext *cx, JS::HandleObject proxy, JS::HandleId id) const; - virtual bool slice(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, - HandleObject result) const; + virtual bool getElements(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, + ElementAdder *adder) const; /* See comment for weakmapKeyDelegateOp in js/Class.h. */ virtual JSObject *weakmapKeyDelegate(JSObject *proxy) const; diff --git a/js/src/proxy/BaseProxyHandler.cpp b/js/src/proxy/BaseProxyHandler.cpp index ff0454427680..403e5f856f01 100644 --- a/js/src/proxy/BaseProxyHandler.cpp +++ b/js/src/proxy/BaseProxyHandler.cpp @@ -341,12 +341,12 @@ BaseProxyHandler::unwatch(JSContext *cx, HandleObject proxy, HandleId id) const } bool -BaseProxyHandler::slice(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, - HandleObject result) const +BaseProxyHandler::getElements(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, + ElementAdder *adder) const { assertEnteredPolicy(cx, proxy, JSID_VOID, GET); - return js::SliceSlowly(cx, proxy, proxy, begin, end, result); + return js::GetElementsWithAdder(cx, proxy, proxy, begin, end, adder); } bool diff --git a/js/src/proxy/Proxy.cpp b/js/src/proxy/Proxy.cpp index 67d92da466de..505740fde96a 100644 --- a/js/src/proxy/Proxy.cpp +++ b/js/src/proxy/Proxy.cpp @@ -550,8 +550,8 @@ Proxy::unwatch(JSContext *cx, JS::HandleObject proxy, JS::HandleId id) } /* static */ bool -Proxy::slice(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, - HandleObject result) +Proxy::getElements(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, + ElementAdder *adder) { JS_CHECK_RECURSION(cx, return false); const BaseProxyHandler *handler = proxy->as().handler(); @@ -560,11 +560,11 @@ Proxy::slice(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, if (!policy.allowed()) { if (policy.returnValue()) { MOZ_ASSERT(!cx->isExceptionPending()); - return js::SliceSlowly(cx, proxy, proxy, begin, end, result); + return js::GetElementsWithAdder(cx, proxy, proxy, begin, end, adder); } return false; } - return handler->slice(cx, proxy, begin, end, result); + return handler->getElements(cx, proxy, begin, end, adder); } JSObject * @@ -835,10 +835,10 @@ js::proxy_Unwatch(JSContext *cx, HandleObject obj, HandleId id) } bool -js::proxy_Slice(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, - HandleObject result) +js::proxy_GetElements(JSContext *cx, HandleObject proxy, uint32_t begin, uint32_t end, + ElementAdder *adder) { - return Proxy::slice(cx, proxy, begin, end, result); + return Proxy::getElements(cx, proxy, begin, end, adder); } const Class js::ProxyObject::class_ = diff --git a/js/src/proxy/Proxy.h b/js/src/proxy/Proxy.h index f163c52cda67..c634a040a5fb 100644 --- a/js/src/proxy/Proxy.h +++ b/js/src/proxy/Proxy.h @@ -69,8 +69,8 @@ class Proxy static bool watch(JSContext *cx, HandleObject proxy, HandleId id, HandleObject callable); static bool unwatch(JSContext *cx, HandleObject proxy, HandleId id); - static bool slice(JSContext *cx, HandleObject obj, uint32_t begin, uint32_t end, - HandleObject result); + static bool getElements(JSContext *cx, HandleObject obj, uint32_t begin, uint32_t end, + ElementAdder *adder); /* IC entry path for handling __noSuchMethod__ on access. */ static bool callProp(JSContext *cx, HandleObject proxy, HandleObject reveiver, HandleId id, diff --git a/js/src/vm/ScopeObject.cpp b/js/src/vm/ScopeObject.cpp index f4fdfc2b21e6..1c6ea12f9b8f 100644 --- a/js/src/vm/ScopeObject.cpp +++ b/js/src/vm/ScopeObject.cpp @@ -605,7 +605,7 @@ const Class DynamicWithObject::class_ = { with_SetGenericAttributes, with_DeleteGeneric, nullptr, nullptr, /* watch/unwatch */ - nullptr, /* slice */ + nullptr, /* getElements */ nullptr, /* enumerate (native enumeration of target doesn't work) */ with_ThisObject, } @@ -1045,7 +1045,7 @@ const Class UninitializedLexicalObject::class_ = { uninitialized_SetGenericAttributes, uninitialized_DeleteGeneric, nullptr, nullptr, /* watch/unwatch */ - nullptr, /* slice */ + nullptr, /* getElements */ nullptr, /* enumerate (native enumeration of target doesn't work) */ nullptr, /* this */ } diff --git a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp index 762747134a2d..471c27d86dd6 100644 --- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp +++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp @@ -697,7 +697,7 @@ const XPCWrappedNativeJSClass XPC_WN_NoHelper_JSClass = { nullptr, // setGenericAttributes nullptr, // deleteGeneric nullptr, nullptr, // watch/unwatch - nullptr, // slice + nullptr, // getElements XPC_WN_JSOp_Enumerate, XPC_WN_JSOp_ThisObject, } diff --git a/js/xpconnect/src/xpcprivate.h b/js/xpconnect/src/xpcprivate.h index f4c729c2bf83..f73634d612c1 100644 --- a/js/xpconnect/src/xpcprivate.h +++ b/js/xpconnect/src/xpcprivate.h @@ -974,7 +974,7 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS::HandleObject obj); nullptr, /* setGenericAttributes */ \ nullptr, /* deleteGeneric */ \ nullptr, nullptr, /* watch/unwatch */ \ - nullptr, /* slice */ \ + nullptr, /* getElements */ \ XPC_WN_JSOp_Enumerate, \ XPC_WN_JSOp_ThisObject, \ } @@ -997,7 +997,7 @@ XPC_WN_JSOp_ThisObject(JSContext *cx, JS::HandleObject obj); nullptr, /* setGenericAttributes */ \ nullptr, /* deleteGeneric */ \ nullptr, nullptr, /* watch/unwatch */ \ - nullptr, /* slice */ \ + nullptr, /* getElements */ \ XPC_WN_JSOp_Enumerate, \ XPC_WN_JSOp_ThisObject, \ }