Commit 70eeb22d authored by Simon Zünd's avatar Simon Zünd Committed by Commit Bot

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

This reverts commit 2b0ac2fb.

Reason for revert: Breaks scrollingcoordinator/non-fast-scrollable-region-nested.html layout test on https://ci.chromium.org/p/v8/builders/ci/V8-Blink%20Linux%2064/32241 

Original change's description:
> [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: Tobias Tebbi <tebbi@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#61812}

TBR=peter.wm.wong@gmail.com,jgruber@chromium.org,tebbi@chromium.org,szuend@chromium.org

Change-Id: If1c1bc07f38dfbd4bf6b6ce8f9d70714e7526877
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:8714
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1627976Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
Commit-Queue: Simon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61814}
parent b851d753
......@@ -1024,7 +1024,6 @@ 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,6 +263,7 @@ bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
V(HasFastPackedElements) \
V(NewArray) \
V(NormalizeElements) \
V(PrepareElementsForSort) \
V(TypedArrayGetBuffer) \
/* Errors */ \
V(NewTypeError) \
......@@ -512,6 +513,7 @@ 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:
......@@ -785,7 +787,6 @@ 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,6 +140,8 @@ 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,6 +1963,17 @@ 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,6 +259,9 @@ 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,6 +3811,20 @@ 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,6 +46,7 @@ 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,9 +93,7 @@ function CreateHoleyObjectArray() {
function CreateDictionaryArray() {
array_to_sort = Array.from(template_array);
Object.defineProperty(array_to_sort, kArraySize - 2,
{ get: () => this.foo,
set: (v) => this.foo = v });
array_to_sort[%MaxSmi()] = 42;
AssertDictionaryElements();
}
......
......@@ -127,8 +127,9 @@ 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) {
......@@ -146,7 +147,8 @@ function TestArrayLongerLength(length) {
TestArrayLongerLength(4);
TestArrayLongerLength(10);
TestArrayLongerLength(1000);
TestArrayLongerLength(5000);
TestArrayLongerLength(500000);
TestArrayLongerLength(Math.pow(2,32) - 1);
function TestNonArrayLongerLength(length) {
......@@ -164,7 +166,8 @@ function TestNonArrayLongerLength(length) {
TestNonArrayLongerLength(4);
TestNonArrayLongerLength(10);
TestNonArrayLongerLength(1000);
TestNonArrayLongerLength(5000);
TestNonArrayLongerLength(500000);
TestNonArrayLongerLength(Math.pow(2,32) - 1);
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(1, xs[0]);
assertEquals(2, xs[1]);
assertEquals(2, xs[0]);
assertEquals(1, xs[1]);
......@@ -72,3 +72,21 @@ 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,13 +163,6 @@
# 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
......@@ -516,6 +509,7 @@
'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,11 +36,6 @@
# 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