Bug 1383648 - Move the Array.prototype.sort entry point to self-hosted code. r=jandem
authorAndré Bargull <andre.bargull@gmail.com>
Wed, 16 Aug 2017 14:08:37 +0200
changeset 647793 5f199b9a3f473c9afe7af7ae0f87aadb2a45f2c0
parent 647792 6eedff8e95498ab746581f3daaab5563dc84f37a
child 647794 8dfe4d26c70f4a792b429186c8ece3d1164efdc8
push id74542
push usermaglione.k@gmail.com
push dateWed, 16 Aug 2017 23:10:03 +0000
reviewersjandem
bugs1383648
milestone57.0a1
Bug 1383648 - Move the Array.prototype.sort entry point to self-hosted code. r=jandem
js/src/builtin/Array.js
js/src/builtin/Intl.js
js/src/builtin/Module.js
js/src/jsarray.cpp
js/src/jsarray.h
js/src/vm/SelfHosting.cpp
--- a/js/src/builtin/Array.js
+++ b/js/src/builtin/Array.js
@@ -180,27 +180,34 @@ function ArrayStaticSome(list, callbackf
     var T = arguments.length > 2 ? arguments[2] : void 0;
     return callFunction(ArraySome, list, callbackfn, T);
 }
 
 // ES2018 draft rev 3bbc87cd1b9d3bf64c3e68ca2fe9c5a3f2c304c0
 // 22.1.3.25 Array.prototype.sort ( comparefn )
 function ArraySort(comparefn) {
     // Step 1.
-    assert(typeof comparefn === "function", "Only called when a comparator is present");
+    if (comparefn !== undefined) {
+        if (!IsCallable(comparefn))
+            ThrowTypeError(JSMSG_BAD_SORT_ARG);
+    }
 
     // Step 2.
-    assert(IsObject(this), "|this| should be an object");
-    var O = this;
+    var O = ToObject(this);
+
+    // First try to sort the array in native code, if that fails, indicated by
+    // returning |false| from ArrayNativeSort, sort it in self-hosted code.
+    if (callFunction(ArrayNativeSort, O, comparefn))
+        return O;
 
     // Step 3.
     var len = ToLength(O.length);
 
     if (len <= 1)
-      return this;
+      return O;
 
     /* 22.1.3.25.1 Runtime Semantics: SortCompare( x, y ) */
     var wrappedCompareFn = comparefn;
     comparefn = function(x, y) {
         /* Steps 1-3. */
         if (x === undefined) {
             if (y === undefined)
                 return 0;
@@ -1114,17 +1121,17 @@ function ArrayStaticReverse(arr) {
     if (arguments.length < 1)
         ThrowTypeError(JSMSG_MISSING_FUN_ARG, 0, "Array.reverse");
     return callFunction(std_Array_reverse, arr);
 }
 
 function ArrayStaticSort(arr, comparefn) {
     if (arguments.length < 1)
         ThrowTypeError(JSMSG_MISSING_FUN_ARG, 0, "Array.sort");
-    return callFunction(std_Array_sort, arr, comparefn);
+    return callFunction(ArraySort, arr, comparefn);
 }
 
 function ArrayStaticPush(arr, arg1) {
     if (arguments.length < 1)
         ThrowTypeError(JSMSG_MISSING_FUN_ARG, 0, "Array.push");
     var args = callFunction(std_Array_slice, arguments, 1);
     return callFunction(std_Function_apply, std_Array_push, arr, args);
 }
--- a/js/src/builtin/Intl.js
+++ b/js/src/builtin/Intl.js
@@ -450,17 +450,17 @@ function CanonicalizeLanguageTag(locale)
     while (i < subtags.length && subtags[i] !== "x") {
         var extensionStart = i;
         i++;
         while (i < subtags.length && subtags[i].length > 1)
             i++;
         var extension = ArrayJoinRange(subtags, "-", extensionStart, i);
         _DefineDataProperty(extensions, extensions.length, extension);
     }
-    callFunction(std_Array_sort, extensions);
+    callFunction(ArraySort, extensions);
 
     // Private use sequences are left as is. "x-private"
     var privateUse = "";
     if (i < subtags.length)
         privateUse = ArrayJoinRange(subtags, "-", i);
 
     // Put everything back together.
     var canonical = normal;
--- a/js/src/builtin/Module.js
+++ b/js/src/builtin/Module.js
@@ -166,17 +166,17 @@ function GetModuleNamespace(module)
 
     // Step 4
     return namespace;
 }
 
 // 9.4.6.13 ModuleNamespaceCreate(module, exports)
 function ModuleNamespaceCreate(module, exports)
 {
-    callFunction(std_Array_sort, exports);
+    callFunction(ArraySort, exports);
 
     let ns = NewModuleNamespace(module, exports);
 
     // Pre-compute all bindings now rather than calling ResolveExport() on every
     // access.
     for (let i = 0; i < exports.length; i++) {
         let name = exports[i];
         let binding = callFunction(module.resolveExport, module, name);
--- a/js/src/jsarray.cpp
+++ b/js/src/jsarray.cpp
@@ -1882,26 +1882,22 @@ enum ComparatorMatchResult {
 } // namespace
 
 
 /*
  * Specialize behavior for comparator functions with particular common bytecode
  * patterns: namely, |return x - y| and |return y - x|.
  */
 static ComparatorMatchResult
-MatchNumericComparator(JSContext* cx, const Value& v)
+MatchNumericComparator(JSContext* cx, JSObject* obj)
 {
-    if (!v.isObject())
+    if (!obj->is<JSFunction>())
         return Match_None;
 
-    JSObject& obj = v.toObject();
-    if (!obj.is<JSFunction>())
-        return Match_None;
-
-    RootedFunction fun(cx, &obj.as<JSFunction>());
+    RootedFunction fun(cx, &obj->as<JSFunction>());
     if (!fun->isInterpreted() || fun->isClassConstructor())
         return Match_None;
 
     JSScript* script = JSFunction::getOrCreateScript(cx, fun);
     if (!script)
         return Match_Failure;
 
     jsbytecode* pc = script->code();
@@ -2094,58 +2090,51 @@ FillWithUndefined(JSContext* cx, HandleO
         if (!CheckForInterrupt(cx) || !SetArrayElement(cx, obj, start + i, UndefinedHandleValue))
             return false;
     }
 
     return true;
 }
 
 bool
-js::array_sort(JSContext* cx, unsigned argc, Value* vp)
+js::intrinsic_ArrayNativeSort(JSContext* cx, unsigned argc, Value* vp)
 {
+    // This function is called from the self-hosted Array.prototype.sort
+    // implementation. It returns |true| if the array was sorted, otherwise it
+    // returns |false| to notify the self-hosted code to perform the sorting.
     CallArgs args = CallArgsFromVp(argc, vp);
-
-    RootedValue fval(cx);
-
-    if (args.hasDefined(0)) {
-        if (!IsCallable(args[0])) {
-            JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_BAD_SORT_ARG);
+    MOZ_ASSERT(args.length() == 1);
+
+    HandleValue fval = args[0];
+    MOZ_ASSERT(fval.isUndefined() || IsCallable(fval));
+
+    ComparatorMatchResult comp;
+    if (fval.isObject()) {
+        comp = MatchNumericComparator(cx, &fval.toObject());
+        if (comp == Match_Failure)
             return false;
+
+        if (comp == Match_None) {
+            // Non-optimized user supplied comparators perform much better when
+            // called from within a self-hosted sorting function.
+            args.rval().setBoolean(false);
+            return true;
         }
-        fval = args[0];     /* non-default compare function */
     } else {
-        fval.setNull();
+        comp = Match_None;
     }
 
-    RootedObject obj(cx, ToObject(cx, args.thisv()));
-    if (!obj)
-        return false;
-
-    ComparatorMatchResult comp = MatchNumericComparator(cx, fval);
-    if (comp == Match_Failure)
-        return false;
-
-    if (!fval.isNull() && comp == Match_None) {
-        /*
-         * Non-optimized user supplied comparators perform much better when
-         * called from within a self-hosted sorting function.
-         */
-        FixedInvokeArgs<1> args2(cx);
-        args2[0].set(fval);
-
-        RootedValue thisv(cx, ObjectValue(*obj));
-        return CallSelfHostedFunction(cx, cx->names().ArraySort, thisv, args2, args.rval());
-    }
+    RootedObject obj(cx, &args.thisv().toObject());
 
     uint64_t length;
     if (!GetLengthProperty(cx, obj, &length))
         return false;
     if (length < 2) {
         /* [] and [a] remain unchanged when sorted. */
-        args.rval().setObject(*obj);
+        args.rval().setBoolean(true);
         return true;
     }
 
     if (length > UINT32_MAX) {
         ReportAllocationOverflow(cx);
         return false;
     }
     uint32_t len = uint32_t(length);
@@ -2223,22 +2212,22 @@ js::array_sort(JSContext* cx, unsigned a
         }
 
         /*
          * If the array only contains holes, we're done.  But if it contains
          * undefs, those must be sorted to the front of the array.
          */
         n = vec.length();
         if (n == 0 && undefs == 0) {
-            args.rval().setObject(*obj);
+            args.rval().setBoolean(true);
             return true;
         }
 
         /* Here len == n + undefs + number_of_holes. */
-        if (fval.isNull()) {
+        if (comp == Match_None) {
             /*
              * Sort using the default comparator converting all elements to
              * strings.
              */
             if (allStrings) {
                 JS_ALWAYS_TRUE(vec.resize(n * 2));
                 if (!MergeSort(vec.begin(), n, vec.begin() + n, SortComparatorStrings(cx)))
                     return false;
@@ -2277,17 +2266,17 @@ js::array_sort(JSContext* cx, unsigned a
         n += undefs;
     }
 
     /* Re-create any holes that sorted to the end of the array. */
     while (len > n) {
         if (!CheckForInterrupt(cx) || !DeletePropertyOrThrow(cx, obj, --len))
             return false;
     }
-    args.rval().setObject(*obj);
+    args.rval().setBoolean(true);
     return true;
 }
 
 bool
 js::NewbornArrayPush(JSContext* cx, HandleObject obj, const Value& v)
 {
     Rooted<ArrayObject*> arr(cx, &obj->as<ArrayObject>());
 
@@ -3509,17 +3498,17 @@ static const JSFunctionSpec array_method
     JS_FN(js_toSource_str,      array_toSource,     0,0),
 #endif
     JS_SELF_HOSTED_FN(js_toString_str, "ArrayToString",      0,0),
     JS_FN(js_toLocaleString_str,       array_toLocaleString, 0,0),
 
     /* Perl-ish methods. */
     JS_INLINABLE_FN("join",     array_join,         1,0, ArrayJoin),
     JS_FN("reverse",            array_reverse,      0,0),
-    JS_FN("sort",               array_sort,         1,0),
+    JS_SELF_HOSTED_FN("sort",   "ArraySort",        1,0),
     JS_INLINABLE_FN("push",     array_push,         1,0, ArrayPush),
     JS_INLINABLE_FN("pop",      array_pop,          0,0, ArrayPop),
     JS_INLINABLE_FN("shift",    array_shift,        0,0, ArrayShift),
     JS_FN("unshift",            array_unshift,      1,0),
     JS_FNINFO("splice",         array_splice,       &array_splice_info, 2,0),
 
     /* Pythonic sequence methods. */
     JS_SELF_HOSTED_FN("concat",      "ArrayConcat",      1,0),
--- a/js/src/jsarray.h
+++ b/js/src/jsarray.h
@@ -153,17 +153,17 @@ SetLengthProperty(JSContext* cx, HandleO
  * GetLengthProperty on aobj. vp must point to rooted memory.
  */
 extern bool
 GetElements(JSContext* cx, HandleObject aobj, uint32_t length, js::Value* vp);
 
 /* Natives exposed for optimization by the interpreter and JITs. */
 
 extern bool
-array_sort(JSContext* cx, unsigned argc, js::Value* vp);
+intrinsic_ArrayNativeSort(JSContext* cx, unsigned argc, js::Value* vp);
 
 extern bool
 array_push(JSContext* cx, unsigned argc, js::Value* vp);
 
 extern bool
 array_pop(JSContext* cx, unsigned argc, js::Value* vp);
 
 extern bool
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2155,19 +2155,19 @@ intrinsic_AddPromiseReactions(JSContext*
 static const JSFunctionSpec intrinsic_functions[] = {
     JS_INLINABLE_FN("std_Array",                 array_construct,              1,0, Array),
     JS_FN("std_Array_join",                      array_join,                   1,0),
     JS_INLINABLE_FN("std_Array_push",            array_push,                   1,0, ArrayPush),
     JS_INLINABLE_FN("std_Array_pop",             array_pop,                    0,0, ArrayPop),
     JS_INLINABLE_FN("std_Array_shift",           array_shift,                  0,0, ArrayShift),
     JS_FN("std_Array_unshift",                   array_unshift,                1,0),
     JS_INLINABLE_FN("std_Array_slice",           array_slice,                  2,0, ArraySlice),
-    JS_FN("std_Array_sort",                      array_sort,                   1,0),
     JS_FN("std_Array_reverse",                   array_reverse,                0,0),
     JS_FNINFO("std_Array_splice",                array_splice, &array_splice_info, 2,0),
+    JS_FN("ArrayNativeSort",                     intrinsic_ArrayNativeSort,    1,0),
 
     JS_FN("std_Date_now",                        date_now,                     0,0),
     JS_FN("std_Date_valueOf",                    date_valueOf,                 0,0),
 
     JS_FN("std_Function_apply",                  fun_apply,                    2,0),
 
     JS_INLINABLE_FN("std_Math_floor",            math_floor,                   1,0, MathFloor),
     JS_INLINABLE_FN("std_Math_max",              math_max,                     2,0, MathMax),