Commit 2b0ac2fb authored by Simon Zünd's avatar Simon Zünd Committed by Commit Bot

[array] Move Array#sort pre-processing to Torque

This CL removes the "PrepareElementsForSort" runtime function, and
replaces it with a simpler version in Torque. The biggest difference
is that certain sparse configurations no longer have a fast-path.

The Torque pre-processing step replaces the existing Torque mechanism that
copied already pre-processed elements into the "work" FixedArray. The Torque
compacting works as follows:
  - Iterate all elements from 0 to {length}
    - If the element is the hole: Do nothing.
    - If the element is "undefined": Increment undefined counter.
    - In all other cases, push the element into the "work" FixedArray.

Then the "work" FixedArray is sorted as before. Writing the elements from
the "work" array back into the receiver, after sorting, has three steps:
  1. Copy the sorted elements from the "work" FixedArray to the receiver.
  2. Add previously counted number of "undefined" to the receiver.
  3. Depending on the backing store either delete properties or
     set them to the Hole up to {length}.

Bug: v8:8714
Change-Id: I14eccb7cfd2e4618bce2a85cba0689d7e0380ad2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1619756
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61812}
parent afb0d4bc
......@@ -1024,6 +1024,7 @@ const V8_TYPED_ARRAY_MAX_SIZE_IN_HEAP:
constexpr int31 generates 'V8_TYPED_ARRAY_MAX_SIZE_IN_HEAP';
const kMaxSafeInteger: constexpr float64 generates 'kMaxSafeInteger';
const kSmiMaxValue: constexpr uintptr generates 'kSmiMaxValue';
const kSmiMax: uintptr = kSmiMaxValue;
const kStringMaxLength: constexpr int31 generates 'String::kMaxLength';
const kFixedArrayMaxLength:
constexpr int31 generates 'FixedArray::kMaxLength';
......
......@@ -263,7 +263,6 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
V(HasFastPackedElements) \
V(NewArray) \
V(NormalizeElements) \
V(PrepareElementsForSort) \
V(TypedArrayGetBuffer) \
/* Errors */ \
V(NewTypeError) \
......@@ -513,7 +512,6 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) {
case Builtins::kArrayPrototypeKeys:
case Builtins::kArrayPrototypeLastIndexOf:
case Builtins::kArrayPrototypeSlice:
case Builtins::kArrayPrototypeSort:
case Builtins::kArrayPrototypeToLocaleString:
case Builtins::kArrayPrototypeToString:
case Builtins::kArrayForEach:
......@@ -787,6 +785,7 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) {
case Builtins::kArrayPrototypeReverse:
case Builtins::kArrayPrototypeShift:
case Builtins::kArrayPrototypeUnshift:
case Builtins::kArrayPrototypeSort:
case Builtins::kArrayPrototypeSplice:
case Builtins::kArrayUnshift:
// Map builtins.
......
......@@ -140,8 +140,6 @@ class FixedArray : public FixedArrayBase {
inline ObjectSlot GetFirstElementAddress();
inline bool ContainsOnlySmisOrHoles();
// Returns true iff the elements are Numbers and sorted ascending.
bool ContainsSortedNumbers();
// Gives access to raw memory which stores the array's data.
inline ObjectSlot data_start();
......
......@@ -1963,17 +1963,6 @@ MaybeHandle<FixedArray> JSReceiver::GetOwnEntries(Handle<JSReceiver> object,
try_fast_path, true);
}
Handle<FixedArray> JSReceiver::GetOwnElementIndices(Isolate* isolate,
Handle<JSReceiver> receiver,
Handle<JSObject> object) {
KeyAccumulator accumulator(isolate, KeyCollectionMode::kOwnOnly,
ALL_PROPERTIES);
accumulator.CollectOwnElementIndices(receiver, object);
Handle<FixedArray> keys =
accumulator.GetKeys(GetKeysConversion::kKeepNumbers);
DCHECK(keys->ContainsSortedNumbers());
return keys;
}
Maybe<bool> JSReceiver::SetPrototype(Handle<JSReceiver> object,
Handle<Object> value, bool from_javascript,
ShouldThrow should_throw) {
......
......@@ -259,9 +259,6 @@ class JSReceiver : public HeapObject {
Handle<JSReceiver> object, PropertyFilter filter,
bool try_fast_path = true);
V8_WARN_UNUSED_RESULT static Handle<FixedArray> GetOwnElementIndices(
Isolate* isolate, Handle<JSReceiver> receiver, Handle<JSObject> object);
static const int kHashMask = PropertyArray::HashField::kMask;
DEFINE_FIELD_OFFSET_CONSTANTS(HeapObject::kHeaderSize,
......
......@@ -3811,20 +3811,6 @@ Handle<FixedArray> FixedArray::SetAndGrow(Isolate* isolate,
return new_array;
}
bool FixedArray::ContainsSortedNumbers() {
for (int i = 1; i < length(); ++i) {
Object a_obj = get(i - 1);
Object b_obj = get(i);
if (!a_obj.IsNumber() || !b_obj.IsNumber()) return false;
uint32_t a = NumberToUint32(a_obj);
uint32_t b = NumberToUint32(b_obj);
if (a > b) return false;
}
return true;
}
Handle<FixedArray> FixedArray::ShrinkOrEmpty(Isolate* isolate,
Handle<FixedArray> array,
int new_length) {
......
This diff is collapsed.
......@@ -46,7 +46,6 @@ namespace internal {
I(IsArray, 1, 1) \
F(NewArray, -1 /* >= 3 */, 1) \
F(NormalizeElements, 1, 1) \
F(PrepareElementsForSort, 2, 1) \
F(TransitionElementsKind, 2, 1) \
F(TransitionElementsKindWithKind, 2, 1) \
......
......@@ -93,7 +93,9 @@ function CreateHoleyObjectArray() {
function CreateDictionaryArray() {
array_to_sort = Array.from(template_array);
array_to_sort[%MaxSmi()] = 42;
Object.defineProperty(array_to_sort, kArraySize - 2,
{ get: () => this.foo,
set: (v) => this.foo = v });
AssertDictionaryElements();
}
......
......@@ -127,9 +127,8 @@ function TestSparseNonArraySorting(length) {
assertFalse(4 in obj, "objsort non-existing retained");
}
TestSparseNonArraySorting(1000);
TestSparseNonArraySorting(5000);
TestSparseNonArraySorting(500000);
TestSparseNonArraySorting(Math.pow(2, 31) + 1);
function TestArrayLongerLength(length) {
......@@ -147,8 +146,7 @@ function TestArrayLongerLength(length) {
TestArrayLongerLength(4);
TestArrayLongerLength(10);
TestArrayLongerLength(1000);
TestArrayLongerLength(500000);
TestArrayLongerLength(Math.pow(2,32) - 1);
TestArrayLongerLength(5000);
function TestNonArrayLongerLength(length) {
......@@ -166,8 +164,7 @@ function TestNonArrayLongerLength(length) {
TestNonArrayLongerLength(4);
TestNonArrayLongerLength(10);
TestNonArrayLongerLength(1000);
TestNonArrayLongerLength(500000);
TestNonArrayLongerLength(Math.pow(2,32) - 1);
TestNonArrayLongerLength(5000);
function TestNonArrayWithAccessors() {
......
......@@ -22,5 +22,5 @@ Array.prototype.sort.call(xs);
// the spec:
// - "xs" is sparse and IsExtensible(xs) is false (its frozen).
// - "xs" is sparse and the prototype has properties in the sort range.
assertEquals(2, xs[0]);
assertEquals(1, xs[1]);
assertEquals(1, xs[0]);
assertEquals(2, xs[1]);
......@@ -72,21 +72,3 @@ float_array[0] = 1e51;
%OptimizeFunctionOnNextCall(f);
f();
})();
// crbug.com/935133
(function() {
var called_has = false;
var proxy = new Proxy({}, {
has: function(x, p) {
called_has = true;
throw "The test may finish now";
},
});
proxy.length = 2147483648;
try {
Array.prototype.sort.call(proxy);
} catch(e) {
assertTrue(e === "The test may finish now");
}
assertTrue(called_has);
})();
......@@ -163,6 +163,13 @@
# https://crbug.com/v8/8120
'ecma_3/Array/regress-322135-04': [SKIP],
# These tests try to sort very large arrays. Array#sort pre-processing does
# not support huge sparse Arrays, so these tests run a very long time.
# https://crbug.com/v8/8714
'js1_5/Array/regress-330812': [SKIP],
'js1_5/Regress/regress-422348': [SKIP],
'js1_5/Array/regress-157652': [SKIP],
##################### SLOW TESTS #####################
# Compiles a long chain of && or || operations, can time out under slower
......@@ -509,7 +516,6 @@
'ecma_3/extensions/regress-274152': [FAIL_OK],
'js1_5/Regress/regress-372364': [FAIL_OK],
'js1_5/Regress/regress-420919': [FAIL_OK],
'js1_5/Regress/regress-422348': [FAIL_OK],
'js1_5/Regress/regress-410852': [FAIL_OK],
'ecma_3/RegExp/regress-375715-04': [FAIL_OK],
'js1_5/decompilation/regress-456964-01': [FAIL_OK],
......
......@@ -36,6 +36,11 @@
# Irregexp interpreter overflows stack. We should just not crash.
'fast/js/regexp-stack-overflow': [PASS, FAIL],
# This test tries to sort very large array. Array#sort pre-processing does
# not support huge sparse Arrays, so this test runs a very long time.
# https://crbug.com/v8/8714
'array-sort-small-sparse-array-with-large-length': [SKIP],
# Slow tests.
'dfg-double-vote-fuzz': [PASS, SLOW],
}], # ALWAYS
......
This diff is collapsed.
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