Revert "[array] Use random middle element to determine pivot during sorting"

This reverts commit 91bab558.

Reason for revert: Seems to break a layout test:
https://ci.chromium.org/buildbot/client.v8.fyi/V8-Blink%20Linux%2064/23895

See also:
https://github.com/v8/v8/wiki/Blink-layout-tests

Original change's description:
> [array] Use random middle element to determine pivot during sorting
> 
> This CL adds a "random state" to the Smi Root list and implements a
> basic Linear congruential pseudo random number generator in Torque.
> 
> The RNG is used to determine the pivot element for sorting. This will
> prevent the worst cases for certain data layouts.
> 
> Drive-by-fix: Make sorting of ranges and execution pauses for profviz
> deterministic by adding a secondary sorting criteria.
> 
> Bug: v8:7382
> Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
> Change-Id: Ieb871e98e74bdb803f821b0cd35d2f67ee0f2868
> Reviewed-on: https://chromium-review.googlesource.com/1082193
> Reviewed-by: Hannes Payer <hpayer@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Commit-Queue: Simon Zünd <szuend@google.com>
> Cr-Commit-Position: refs/heads/master@{#53524}

TBR=hpayer@chromium.org,cbruni@chromium.org,jgruber@chromium.org,szuend@google.com

Change-Id: I54f5d3f719428fd089ff12ff217d1c819f9ad1f7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7382
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1088506Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53542}

src/builtins/array.tq

@@ -637,42 +637,16 @@ module array {
     }
   }
 
-  // Returns a random positive Smi in the range of [0, range).
-  macro Rand(range: Smi): Smi {
-    assert(TaggedIsPositiveSmi(range));
-
-    let current_state: int32 =
-        LoadAndUntagToWord32Root(kArraySortRandomStateRootIndex);
-    let a: int32 = 1103515245;
-    let c: int32 = 12345;
-    let m: int32 = 0x3fffffff; // 2^30 bitmask.
-
-    let new_state: int32 = ((current_state * a) + c) & m;
-    StoreRoot(kArraySortRandomStateRootIndex, convert<Smi>(new_state));
-
-    let r: int32 = convert<int32>(range);
-    return convert<Smi>(new_state % r);
-  }
-
   macro CalculatePivot<E : type>(
       context: Context, receiver: Object, elements: Object,
       initialReceiverMap: Object, initialReceiverLength: Number, from: Smi,
       to: Smi, userCmpFn: Object, sortCompare: CompareBuiltinFn): Object
   labels Bailout {
-    let random: Smi = Rand(to - from - 2);
-    assert(TaggedIsPositiveSmi(random));
-
-    let third_index: Smi = from + 1 + random;
-    assert(third_index > from);
-    assert(third_index <= to - 1);
-
-    // Find a pivot as the median of first, last and a random middle element.
-    // Always using the middle element as the third index causes the quicksort
-    // to degrade to O(n^2) for certain data configurations.
-    // The previous solution was to sample larger arrays and use the median
-    // element of the sorted sample. This causes more overhead than just
-    // choosing a random middle element, which also mitigates the worst cases
-    // in all relevant benchmarks.
+    // TODO(szuend): Check if a more involved third_index calculation is
+    //               worth it for very large arrays.
+    let third_index: Smi = from + ((to - from) >>> 1);
+
+    // Find a pivot as the median of first, last and middle element.
     let v0: Object = Load<E>(context, elements, from) otherwise Bailout;
     let v1: Object = Load<E>(context, elements, to - 1) otherwise Bailout;
     let v2: Object = Load<E>(context, elements, third_index) otherwise Bailout;

src/builtins/base.tq

@@ -53,7 +53,6 @@ type LanguageMode generates 'TNode<Smi>' constexpr 'LanguageMode';
 type ExtractFixedArrayFlags generates
 'TNode<Smi>' constexpr 'ExtractFixedArrayFlags';
 type ParameterMode generates 'TNode<Int32T>' constexpr 'ParameterMode';
-type RootListIndex generates 'TNode<Int32T>' constexpr 'Heap::RootListIndex';
 
 type MessageTemplate constexpr 'MessageTemplate';
 type HasPropertyLookupMode constexpr 'HasPropertyLookupMode';
@@ -132,9 +131,6 @@ const kSloppy: constexpr LanguageMode = 'LanguageMode::kSloppy';
 const SMI_PARAMETERS: constexpr ParameterMode = 'SMI_PARAMETERS';
 const INTPTR_PARAMETERS: constexpr ParameterMode = 'INTPTR_PARAMETERS';
 
-const kArraySortRandomStateRootIndex: constexpr RootListIndex =
-    'Heap::kArraySortRandomStateRootIndex';
-
 extern macro Print(Object);
 extern macro DebugBreak();
 extern macro ToInteger_Inline(Context, Object): Number;
@@ -160,10 +156,6 @@ extern runtime CreateDataProperty(Context, Object, Object, Object);
 extern runtime SetProperty(Context, Object, Object, Object, LanguageMode);
 extern runtime DeleteProperty(Context, Object, Object, LanguageMode);
 
-extern macro LoadRoot(constexpr RootListIndex): Object;
-extern macro StoreRoot(constexpr RootListIndex, Object): Object;
-extern macro LoadAndUntagToWord32Root(constexpr RootListIndex): int32;
-
 extern runtime StringEqual(Context, String, String): Oddball;
 extern builtin StringLessThan(Context, String, String): Boolean;
 
@@ -216,18 +208,12 @@ extern operator '!=' macro WordNotEqual(Object, Object): bool;
 
 extern operator '+' macro SmiAdd(Smi, Smi): Smi;
 extern operator '-' macro SmiSub(Smi, Smi): Smi;
-extern operator '&' macro SmiAnd(Smi, Smi): Smi;
 extern operator '>>>' macro SmiShr(Smi, constexpr int31): Smi;
 
 extern operator '+' macro IntPtrAdd(intptr, intptr): intptr;
 extern operator '-' macro IntPtrSub(intptr, intptr): intptr;
 extern operator '>>>' macro WordShr(intptr, intptr): intptr;
 
-extern operator '+' macro Int32Add(int32, int32): int32;
-extern operator '*' macro Int32Mul(int32, int32): int32;
-extern operator '%' macro Int32Mod(int32, int32): int32;
-extern operator '&' macro Word32And(int32, int32): int32;
-
 extern operator '+' macro NumberAdd(Number, Number): Number;
 extern operator '-' macro NumberSub(Number, Number): Number;
 extern operator 'min' macro NumberMin(Number, Number): Number;
@@ -249,7 +235,6 @@ extern operator
 
 extern operator 'is<Smi>' macro TaggedIsSmi(Object): bool;
 extern operator 'isnt<Smi>' macro TaggedIsNotSmi(Object): bool;
-extern macro TaggedIsPositiveSmi(Object): bool;
 
 extern operator
 'cast<>' macro TaggedToJSDataView(Object): JSDataView labels CastError;
@@ -292,7 +277,6 @@ extern operator 'convert<>' macro TruncateWordToWord32(intptr): int32;
 extern operator 'convert<>' macro SmiTag(intptr): Smi;
 extern operator 'convert<>' macro SmiFromInt32(int32): Smi;
 extern operator 'convert<>' macro SmiUntag(Smi): intptr;
-extern operator 'convert<>' macro SmiToInt32(Smi): int32;
 extern operator 'convert<>' macro LoadHeapNumberValue(HeapNumber): float64;
 extern operator 'convert<>' macro ChangeNumberToFloat64(Number): float64;
 

src/heap/heap.cc

@@ -4735,16 +4735,6 @@ bool Heap::SetUp() {
   DCHECK_EQ(Smi::kZero, hash_seed());
   if (FLAG_randomize_hashes) InitializeHashSeed();
 
-  // Set up the random state that is used for random number generation in
-  // Torque.
-  int array_sort_random_seed = FLAG_random_seed;
-  if (array_sort_random_seed == 0) {
-    array_sort_random_seed = isolate()->random_number_generator()->NextInt();
-  }
-  // Cut the seed to valid Smi range.
-  array_sort_random_seed &= ((static_cast<intptr_t>(1) << kSmiShiftSize) - 1);
-  set_array_sort_random_state(Smi::FromInt(array_sort_random_seed));
-
   for (int i = 0; i < static_cast<int>(v8::Isolate::kUseCounterFeatureCount);
        i++) {
     deferred_counters_[i] = 0;

src/heap/heap.h

@@ -300,8 +300,7 @@ using v8::MemoryPressureLevel;
     ConstructStubCreateDeoptPCOffset)                                          \
   V(Smi, construct_stub_invoke_deopt_pc_offset,                                \
     ConstructStubInvokeDeoptPCOffset)                                          \
-  V(Smi, interpreter_entry_return_pc_offset, InterpreterEntryReturnPCOffset)   \
-  V(Smi, array_sort_random_state, ArraySortRandomState)
+  V(Smi, interpreter_entry_return_pc_offset, InterpreterEntryReturnPCOffset)
 
 #define ROOT_LIST(V)  \
   STRONG_ROOT_LIST(V) \

test/mjsunit/tools/profviz-test.default

@@ -1435,8 +1435,8 @@
   "46.74 0.08499999999999375",
   "57.64849999999997 0.08249999999999602",
   "58.316999999999965 0.08099999999999596",
-  "46.37200000000001 0.08050000000000068",
   "23.506 0.08050000000000068",
+  "46.37200000000001 0.08050000000000068",
   "42.70600000000002 0.07900000000000063",
   "129.4124999999999 0.07800000000000296",
   "20.5975 0.07750000000000057",
@@ -1450,16 +1450,16 @@
   "44.917000000000016 0.07200000000000273",
   "25.591500000000003 0.07199999999999918",
   "50.62049999999999 0.07150000000000034",
-  "88.82299999999992 0.07099999999999795",
   "46.621 0.07099999999999795",
+  "88.82299999999992 0.07099999999999795",
   "78.23049999999994 0.0660000000000025",
   "46.060500000000005 0.0659999999999954",
   "50.43099999999999 0.06400000000000006",
   "129.48849999999987 0.06349999999997635",
   "45.55900000000001 0.06150000000000233",
   "19.152 0.06050000000000111",
-  "57.33299999999997 0.060499999999997556",
   "50.20799999999999 0.060499999999997556",
+  "57.33299999999997 0.060499999999997556",
   "68.76649999999995 0.06049999999999045",
   "23.5775 0.059499999999999886",
   "47.135000000000005 0.05850000000000222",
@@ -1468,8 +1468,8 @@
   "21.2695 0.057500000000000995",
   "50.14149999999999 0.05749999999999744",
   "91.96649999999993 0.056500000000013983",
-  "83.63999999999993 0.05649999999999977",
   "57.934999999999974 0.05649999999999977",
+  "83.63999999999993 0.05649999999999977",
   "132.92249999999987 0.05649999999997135",
   "67.59199999999996 0.056000000000011596",
   "99.92199999999991 0.055499999999995",
@@ -1490,9 +1490,9 @@
   "75.46799999999995 0.04950000000000898",
   "45.76150000000001 0.049500000000001876",
   "94.6054999999999 0.04949999999998056",
-  "118.19199999999991 0.048500000000004206",
-  "115.4124999999999 0.048500000000004206",
   "45.97850000000001 0.048500000000004206",
+  "115.4124999999999 0.048500000000004206",
+  "118.19199999999991 0.048500000000004206",
   "49.780499999999996 0.0484999999999971",
   "42.795000000000016 0.04800000000000182",
   "126.59899999999989 0.04749999999999943",
@@ -1512,23 +1512,23 @@
   "52.15399999999998 0.0379999999999967",
   "42.88200000000001 0.0379999999999967",
   "24.430500000000002 0.03750000000000142",
-  "60.08349999999996 0.036499999999996646",
   "23.907 0.036499999999996646",
+  "60.08349999999996 0.036499999999996646",
   "50.32899999999999 0.036000000000001364",
   "42.31450000000002 0.034000000000006025",
   "45.02900000000001 0.032999999999994145",
   "23.189 0.031500000000001194",
   "21.49 0.03049999999999997",
+  "42.83300000000001 0.030000000000001137",
   "58.12149999999997 0.030000000000001137",
   "45.41750000000002 0.030000000000001137",
-  "42.83300000000001 0.030000000000001137",
   "140.89599999999987 0.028999999999996362",
   "2.4490000000000003 0.028500000000000636",
   "52.31499999999998 0.027999999999998693",
   "45.17200000000002 0.027999999999998693",
-  "51.30249999999999 0.027000000000001023",
-  "49.8685 0.027000000000001023",
   "43.632500000000014 0.027000000000001023",
+  "49.8685 0.027000000000001023",
+  "51.30249999999999 0.027000000000001023",
   "21.175 0.026499999999998636",
   "44.82200000000002 0.026000000000003354",
   "22.528 0.02599999999999625",

test/test262/test262.status

@@ -454,13 +454,6 @@
   'built-ins/TypedArrays/ctors/buffer-arg/buffer-arg-bufferbyteoffset-throws-from-modulo-element-size': [FAIL],
   'built-ins/TypedArrays/ctors/buffer-arg/buffer-arg-byteoffset-throws-from-modulo-element-size': [FAIL],
 
-  # Tests assume that the sort order of "same elements" (comparator returns 0)
-  # is deterministic.
-  # https://bugs.chromium.org/p/v8/issues/detail?id=7808
-  'intl402/String/prototype/localeCompare/returns-same-results-as-Collator': [SKIP],
-  'intl402/Collator/prototype/compare/bound-to-collator-instance': [SKIP],
-  'intl402/Collator/ignore-invalid-unicode-ext-values': [SKIP],
-
 ######################## NEEDS INVESTIGATION ###########################
 
   # These test failures are specific to the intl402 suite and need investigation

tools/profviz/composer.js

@@ -176,9 +176,7 @@ function PlotScriptComposer(kResX, kResY, error_output) {
   }
 
   function MergeRanges(ranges) {
-    ranges.sort(function(a, b) {
-      return (a.start == b.start) ? a.end - b.end : a.start - b.start;
-    });
+    ranges.sort(function(a, b) { return a.start - b.start; });
     var result = [];
     var j = 0;
     for (var i = 0; i < ranges.length; i = j) {
@@ -518,13 +516,8 @@ function PlotScriptComposer(kResX, kResY, error_output) {
     // Label the longest pauses.
     execution_pauses =
         RestrictRangesTo(execution_pauses, range_start, range_end);
-    execution_pauses.sort(function(a, b) {
-      if (a.duration() == b.duration() && b.end == a.end)
-        return b.start - a.start;
-
-      return (a.duration() == b.duration())
-          ? b.end - a.end : b.duration() - a.duration();
-    });
+    execution_pauses.sort(
+        function(a, b) { return b.duration() - a.duration(); });
 
     var max_pause_time = execution_pauses.length > 0
         ? execution_pauses[0].duration() : 0;