Commit 3896cdc2 authored by Simon Zünd's avatar Simon Zünd Committed by Commit Bot

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

This is a reland of 91bab558

This CL contains two major changes w.r.t to the original CL:

The random state is removed from the Smi root list and we pre-seed the RNG
on each sort with the length of the array.

To cut down on the length of the arguments list and to keep track of the
random state across recursive calls, we move most of the sort arguments into
a FixedArray and reload from the array for each recursion.

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}

Bug: v8:7382
Change-Id: Ia7bef7ed1c0e904ffe43bc428e702f64f9c6a60b
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1087888Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Commit-Queue: Simon Zünd <szuend@google.com>
Cr-Commit-Position: refs/heads/master@{#53583}
parent 67d449c3
......@@ -637,16 +637,53 @@ module array {
}
}
// TODO(szuend): Replace these with constants when Torque has them.
macro kReceiverIdx(): constexpr int31 { return 0; }
macro kUserCmpFnIdx(): constexpr int31 { return 1; }
macro kSortComparePtrIdx(): constexpr int31 { return 2; }
macro kInitialReceiverMapIdx(): constexpr int31 { return 3; }
macro kInitialReceiverLengthIdx(): constexpr int31 { return 4; }
macro kElementsIdx(): constexpr int31 { return 5; }
macro kRandomStateIdx(): constexpr int31 { return 6; }
// Returns a random positive Smi in the range of [0, range).
macro Rand(sortState: FixedArray, range: Smi): Smi {
assert(TaggedIsPositiveSmi(range));
let current_state_smi: Smi = unsafe_cast<Smi>(sortState[kRandomStateIdx()]);
let current_state: int32 = convert<int32>(current_state_smi);
let a: int32 = 1103515245;
let c: int32 = 12345;
let m: int32 = 0x3fffffff; // 2^30 bitmask.
let new_state: int32 = ((current_state * a) + c) & m;
sortState[kRandomStateIdx()] = 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
sortState: FixedArray, context: Context, receiver: Object,
elements: Object, initialReceiverMap: Object,
initialReceiverLength: Number, from: Smi, to: Smi, userCmpFn: Object,
sortCompare: CompareBuiltinFn): Object
labels Bailout {
// 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 random: Smi = Rand(sortState, 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.
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;
......@@ -708,13 +745,20 @@ module array {
}
macro ArrayQuickSortImpl<E : type>(
context: Context, receiver: Object, elements: Object,
initialReceiverMap: Object, initialReceiverLength: Number, fromArg: Smi,
toArg: Smi, userCmpFn: Object, sortCompare: CompareBuiltinFn)
context: Context, sortState: FixedArray, fromArg: Smi, toArg: Smi)
labels Bailout {
let from: Smi = fromArg;
let to: Smi = toArg;
let receiver: Object = sortState[kReceiverIdx()];
let userCmpFn: Object = sortState[kUserCmpFnIdx()];
let sortCompare: CompareBuiltinFn =
unsafe_cast<CompareBuiltinFn>(sortState[kSortComparePtrIdx()]);
let initialReceiverMap: Object = sortState[kInitialReceiverMapIdx()];
let initialReceiverLength: Number =
unsafe_cast<Number>(sortState[kInitialReceiverLengthIdx()]);
let elements: Object = sortState[kElementsIdx()];
while (to - from > 1) {
if (to - from <= 10) {
ArrayInsertionSort<E>(
......@@ -725,7 +769,7 @@ module array {
}
let pivot: Object = CalculatePivot<E>(
context, receiver, elements, initialReceiverMap,
sortState, context, receiver, elements, initialReceiverMap,
initialReceiverLength, from, to, userCmpFn, sortCompare)
otherwise Bailout;
......@@ -785,54 +829,43 @@ module array {
}
if ((to - high_start) < (low_end - from)) {
ArrayQuickSort<E>(
context, receiver, elements, initialReceiverMap,
initialReceiverLength, high_start, to, userCmpFn, sortCompare);
ArrayQuickSort<E>(context, sortState, high_start, to);
to = low_end;
} else {
ArrayQuickSort<E>(
context, receiver, elements, initialReceiverMap,
initialReceiverLength, from, low_end, userCmpFn, sortCompare);
ArrayQuickSort<E>(context, sortState, from, low_end);
from = high_start;
}
}
}
builtin ArrayQuickSort<ElementsAccessor : type>(
context: Context, receiver: Object, elements: Object,
initialReceiverMap: Object, initialReceiverLength: Number, from: Smi,
to: Smi, userCmpFn: Object, sortCompare: CompareBuiltinFn): Object {
context: Context, sortState: FixedArray, from: Smi, to: Smi): Object {
try {
ArrayQuickSortImpl<ElementsAccessor>(
context, receiver, elements, initialReceiverMap,
initialReceiverLength, from, to, userCmpFn, sortCompare)
ArrayQuickSortImpl<ElementsAccessor>(context, sortState, from, to)
otherwise Slow;
}
label Slow {
ArrayQuickSort<GenericElementsAccessor>(
context, receiver, receiver, initialReceiverMap,
initialReceiverLength, from, to, userCmpFn, sortCompare);
// Generic version uses Set- and GetProperty, replace elements with
// the receiver itself.
sortState[kElementsIdx()] = sortState[kReceiverIdx()];
ArrayQuickSort<GenericElementsAccessor>(context, sortState, from, to);
}
return receiver;
return SmiConstant(0);
}
// The specialization is needed since we would end up in an endless loop
// when the ElementsAccessor fails and bails to the ElementsAccessor again.
ArrayQuickSort<GenericElementsAccessor>(
context: Context, receiver: Object, elements: Object,
initialReceiverMap: Object, initialReceiverLength: Number, from: Smi,
to: Smi, userCmpFn: Object, sortCompare: CompareBuiltinFn): Object {
context: Context, sortState: FixedArray, from: Smi, to: Smi): Object {
try {
ArrayQuickSortImpl<GenericElementsAccessor>(
context, receiver, elements, initialReceiverMap,
initialReceiverLength, from, to, userCmpFn, sortCompare)
ArrayQuickSortImpl<GenericElementsAccessor>(context, sortState, from, to)
otherwise Error;
}
label Error {
// The generic baseline path must not fail.
unreachable;
}
return receiver;
return SmiConstant(0);
}
// For compatibility with JSC, we also sort elements inherited from
......@@ -860,13 +893,26 @@ module array {
}
// 2. Let obj be ? ToObject(this value).
let obj: Object = ToObject(context, receiver);
let obj: JSReceiver = ToObject(context, receiver);
let map: Map = obj.map;
let sort_state: FixedArray =
AllocateFixedArray(PACKED_ELEMENTS, IntPtrConstant(7));
let cmpBuiltin: CompareBuiltinFn =
sort_state[kReceiverIdx()] = obj;
sort_state[kUserCmpFnIdx()] = comparefnObj;
sort_state[kSortComparePtrIdx()] =
comparefnObj != Undefined ? SortCompareUserFn : SortCompareDefault;
sort_state[kInitialReceiverMapIdx()] = map;
// Initialize the remaining fields with Undefined.
// Needed for heap verification.
sort_state[kInitialReceiverLengthIdx()] = Undefined;
sort_state[kElementsIdx()] = Undefined;
sort_state[kRandomStateIdx()] = Undefined;
try {
let a: JSArray = cast<JSArray>(obj) otherwise slow;
let map: Map = a.map;
let elementsKind: ElementsKind = map.elements_kind;
if (!IsFastElementsKind(elementsKind)) goto slow;
......@@ -878,26 +924,23 @@ module array {
// for PACKED_* and handling Undefineds during sorting.
let nofNonUndefined: Smi = PrepareElementsForSort(context, obj, len);
sort_state[kInitialReceiverLengthIdx()] = len;
sort_state[kElementsIdx()] = a.elements;
sort_state[kRandomStateIdx()] = nofNonUndefined;
// TODO(szuend): Extract into IsDoubleElementsKind when bool types are
// fixed in Torque.
if (elementsKind == convert<ElementsKind>(PACKED_DOUBLE_ELEMENTS) ||
elementsKind == convert<ElementsKind>(HOLEY_DOUBLE_ELEMENTS)) {
let elements: FixedDoubleArray =
unsafe_cast<FixedDoubleArray>(a.elements);
ArrayQuickSort<FastDoubleElements>(
context, obj, elements, map, len, 0, nofNonUndefined, comparefnObj,
cmpBuiltin);
context, sort_state, 0, nofNonUndefined);
} else {
let elements: FixedArray = unsafe_cast<FixedArray>(a.elements);
if (elementsKind == convert<ElementsKind>(PACKED_SMI_ELEMENTS)) {
ArrayQuickSort<FastPackedSmiElements>(
context, obj, elements, map, len, 0, nofNonUndefined,
comparefnObj, cmpBuiltin);
context, sort_state, 0, nofNonUndefined);
} else {
ArrayQuickSort<FastSmiOrObjectElements>(
context, obj, elements, map, len, 0, nofNonUndefined,
comparefnObj, cmpBuiltin);
context, sort_state, 0, nofNonUndefined);
}
}
}
......@@ -909,9 +952,12 @@ module array {
if (len < 2) return receiver;
let nofNonUndefined: Smi = PrepareElementsForSort(context, obj, len);
sort_state[kInitialReceiverLengthIdx()] = len;
sort_state[kElementsIdx()] = obj;
sort_state[kRandomStateIdx()] = nofNonUndefined;
ArrayQuickSort<GenericElementsAccessor>(
context, obj, obj, Undefined, len, 0, nofNonUndefined, comparefnObj,
cmpBuiltin);
context, sort_state, 0, nofNonUndefined);
}
return receiver;
......
......@@ -9,7 +9,7 @@ type never generates 'void';
type Tagged generates 'TNode<Object>';
type Smi extends Tagged generates 'TNode<Smi>';
type HeapObject extends Tagged generates 'TNode<HeapObject>';
type Object = Smi | HeapObject;
type Object = Smi|HeapObject;
type int32 generates 'TNode<Int32T>' constexpr 'int32_t';
type uint32 generates 'TNode<Uint32T>' constexpr 'uint32_t';
type intptr generates 'TNode<IntPtrT>' constexpr 'intptr_t';
......@@ -25,14 +25,14 @@ type Context extends HeapObject generates 'TNode<Context>';
type String extends HeapObject generates 'TNode<String>';
type Oddball extends HeapObject generates 'TNode<Oddball>';
type HeapNumber extends HeapObject generates 'TNode<HeapNumber>';
type Number = Smi | HeapNumber;
type Number = Smi|HeapNumber;
type Boolean extends Oddball generates 'TNode<Oddball>';
type JSProxy extends JSReceiver generates 'TNode<JSProxy>';
type JSObject extends JSReceiver generates 'TNode<JSObject>';
type JSArray extends JSObject generates 'TNode<JSArray>';
type JSFunction extends JSObject generates 'TNode<JSFunction>';
type JSBoundFunction extends JSObject generates 'TNode<JSBoundFunction>';
type Callable = JSFunction | JSBoundFunction | JSProxy;
type Callable = JSFunction|JSBoundFunction|JSProxy;
type Map extends HeapObject generates 'TNode<Map>';
type FixedArrayBase extends HeapObject generates 'TNode<FixedArrayBase>';
type FixedArray extends FixedArrayBase generates 'TNode<FixedArray>';
......@@ -54,6 +54,7 @@ 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';
......@@ -149,7 +150,7 @@ extern macro ThrowTypeError(Context, constexpr MessageTemplate, Object): never;
extern macro ArraySpeciesCreate(Context, Object, Number): Object;
extern macro EnsureArrayPushable(Map): ElementsKind labels Bailout;
extern builtin ToObject(Context, Object): Object;
extern builtin ToObject(Context, Object): JSReceiver;
extern macro IsNullOrUndefined(Object): bool;
extern macro IsTheHole(Object): bool;
extern macro IsString(HeapObject): bool;
......@@ -159,6 +160,10 @@ 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;
......@@ -245,6 +250,7 @@ extern operator
extern operator 'is<Smi>' macro TaggedIsSmi(Object): bool;
extern operator 'isnt<Smi>' macro TaggedIsNotSmi(Object): bool;
extern macro TaggedIsPositiveSmi(Object): bool;
extern macro TaggedToJSDataView(Object): JSDataView labels CastError;
extern macro TaggedToHeapObject(Object): HeapObject labels CastError;
......@@ -311,6 +317,7 @@ 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;
......@@ -344,9 +351,14 @@ extern operator '.length=' macro StoreJSArrayLength(JSArray, Smi);
extern operator '.length' macro LoadFixedArrayBaseLength(FixedArrayBase): Smi;
extern operator '[]' macro LoadFixedArrayElement(FixedArray, intptr): Object;
extern operator '[]' macro LoadFixedArrayElement(FixedArray, Smi): Object;
extern operator
'[]' macro LoadFixedArrayElementInt(FixedArray, constexpr int31): Object;
extern operator
'[]=' macro StoreFixedArrayElement(FixedArray, intptr, Object): void;
extern operator
'[]=' macro StoreFixedArrayElementInt(
FixedArray, constexpr int31, Object): void;
extern operator
'[]=' macro StoreFixedArrayElementSmi(FixedArray, Smi, Object): void;
extern macro LoadFixedDoubleArrayElement(FixedDoubleArray, Smi): float64;
......@@ -364,8 +376,7 @@ extern macro IsFastSmiOrTaggedElementsKind(ElementsKind): bool;
extern macro IsFastSmiElementsKind(ElementsKind): bool;
extern macro IsHoleyFastElementsKind(ElementsKind): bool;
extern macro AllocateFixedArray(constexpr ElementsKind, Smi): FixedArray;
extern macro AllocateFixedArray(constexpr ElementsKind, Smi, Map): FixedArray;
extern macro AllocateFixedArray(constexpr ElementsKind, intptr): FixedArray;
extern macro CopyFixedArrayElements(
constexpr ElementsKind, FixedArray, constexpr ElementsKind, FixedArray,
......
......@@ -68,6 +68,15 @@ class ArrayBuiltinsAssembler : public BaseBuiltinsFromDSLAssembler {
void NullPostLoopAction();
// TODO(szuend): Remove once overload resolution is fixed in Torque.
TNode<Object> LoadFixedArrayElementInt(TNode<FixedArray> array, int index) {
return LoadFixedArrayElement(array, index);
}
Node* StoreFixedArrayElementInt(TNode<FixedArray> array, int index,
TNode<Object> value) {
return StoreFixedArrayElement(array, index, value);
}
protected:
TNode<Context> context() { return context_; }
TNode<Object> receiver() { return receiver_; }
......
......@@ -1435,8 +1435,8 @@
"46.74 0.08499999999999375",
"57.64849999999997 0.08249999999999602",
"58.316999999999965 0.08099999999999596",
"23.506 0.08050000000000068",
"46.37200000000001 0.08050000000000068",
"23.506 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",
"46.621 0.07099999999999795",
"88.82299999999992 0.07099999999999795",
"46.621 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",
"50.20799999999999 0.060499999999997556",
"57.33299999999997 0.060499999999997556",
"50.20799999999999 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",
"57.934999999999974 0.05649999999999977",
"83.63999999999993 0.05649999999999977",
"57.934999999999974 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",
"45.97850000000001 0.048500000000004206",
"115.4124999999999 0.048500000000004206",
"118.19199999999991 0.048500000000004206",
"115.4124999999999 0.048500000000004206",
"45.97850000000001 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",
"23.907 0.036499999999996646",
"60.08349999999996 0.036499999999996646",
"23.907 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",
"43.632500000000014 0.027000000000001023",
"49.8685 0.027000000000001023",
"51.30249999999999 0.027000000000001023",
"49.8685 0.027000000000001023",
"43.632500000000014 0.027000000000001023",
"21.175 0.026499999999998636",
"44.82200000000002 0.026000000000003354",
"22.528 0.02599999999999625",
......
......@@ -454,6 +454,13 @@
'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://crbug.com/v8/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
......
......@@ -176,7 +176,9 @@ function PlotScriptComposer(kResX, kResY, error_output) {
}
function MergeRanges(ranges) {
ranges.sort(function(a, b) { return a.start - b.start; });
ranges.sort(function(a, b) {
return (a.start == b.start) ? a.end - b.end : a.start - b.start;
});
var result = [];
var j = 0;
for (var i = 0; i < ranges.length; i = j) {
......@@ -516,8 +518,13 @@ 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) { return b.duration() - a.duration(); });
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();
});
var max_pause_time = execution_pauses.length > 0
? execution_pauses[0].duration() : 0;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment