Revert "[runtime] Keep FAST_SLOPPY_ARGUMENTS packed"

This reverts commit 28930128ce.

Reason for revert: GC stress failures:
https://build.chromium.org/p/client.v8/builders/V8%20Mac%20GC%20Stress/builds/12958

Original change's description:
> [runtime] Keep FAST_SLOPPY_ARGUMENTS packed
> 
> With this CL SloppyArguments immediately go to dictionary elements on 
> deletion, keeping the arguments backing store packed.
> 
> Bug: v8:6251
> Change-Id: I2afa4fb5f0af9942eee0a1606942f5f289539330
> Reviewed-on: https://chromium-review.googlesource.com/480379
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#44857}

TBR=jkummerow@chromium.org,cbruni@chromium.org,v8-reviews@googlegroups.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: I9482bf693a745d1301d068869ddae39f11143827
Reviewed-on: https://chromium-review.googlesource.com/486885
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44863}
This commit is contained in:
Michael Achenbach 2017-04-25 14:43:05 +00:00 committed by Commit Bot
parent f431b597bf
commit ae1fa3daad
9 changed files with 53 additions and 192 deletions

View File

@ -3439,8 +3439,6 @@ class SloppyArgumentsElementsAccessor
uint32_t entry = ArgumentsAccessor::GetEntryForIndexImpl(
isolate, holder, arguments, index, filter);
if (entry == kMaxUInt32) return kMaxUInt32;
// Arguments entries could overlap with the dictionary entries, hence offset
// them by the number of context mapped entries.
return elements->parameter_map_length() + entry;
}
@ -3464,21 +3462,17 @@ class SloppyArgumentsElementsAccessor
}
static void DeleteImpl(Handle<JSObject> obj, uint32_t entry) {
Handle<SloppyArgumentsElements> elements(
SloppyArgumentsElements::cast(obj->elements()));
SloppyArgumentsElements* elements =
SloppyArgumentsElements::cast(obj->elements());
uint32_t length = elements->parameter_map_length();
if (entry < length) {
// TODO(kmillikin): We could check if this was the last aliased
// parameter, and revert to normal elements in that case. That
// would enable GC of the context.
elements->set_mapped_entry(entry, obj->GetHeap()->the_hole_value());
entry = kMaxUInt32;
} else {
Subclass::DeleteFromArguments(obj, entry - length);
}
Subclass::SloppyDeleteImpl(obj, elements, entry);
}
static void SloppyDeleteImpl(Handle<JSObject> obj,
Handle<SloppyArgumentsElements> elements,
uint32_t entry) {
// Implemented in subclasses.
UNREACHABLE();
}
static void CollectElementIndicesImpl(Handle<JSObject> object,
@ -3631,21 +3625,17 @@ class SlowSloppyArgumentsElementsAccessor
}
return result;
}
static void SloppyDeleteImpl(Handle<JSObject> obj,
Handle<SloppyArgumentsElements> elements,
uint32_t entry) {
// No need to delete a context mapped entry from the arguments elements.
if (entry == kMaxUInt32) return;
static void DeleteFromArguments(Handle<JSObject> obj, uint32_t entry) {
Isolate* isolate = obj->GetIsolate();
Handle<SloppyArgumentsElements> elements(
SloppyArgumentsElements::cast(obj->elements()), isolate);
Handle<SeededNumberDictionary> dict(
SeededNumberDictionary::cast(elements->arguments()), isolate);
// TODO(verwaest): Remove reliance on index in Shrink.
uint32_t index = GetIndexForEntryImpl(*dict, entry);
int length = elements->parameter_map_length();
Handle<Object> result =
SeededNumberDictionary::DeleteProperty(dict, entry - length);
Handle<Object> result = SeededNumberDictionary::DeleteProperty(dict, entry);
USE(result);
DCHECK(result->IsTrue(isolate));
DCHECK(result->IsTrue(dict->GetIsolate()));
Handle<FixedArray> new_elements =
SeededNumberDictionary::Shrink(dict, index);
elements->set_arguments(*new_elements);
@ -3769,28 +3759,10 @@ class FastSloppyArgumentsElementsAccessor
return FastHoleyObjectElementsAccessor::NormalizeImpl(object, arguments);
}
static Handle<SeededNumberDictionary> NormalizeArgumentsElements(
Handle<JSObject> object, Handle<SloppyArgumentsElements> elements,
uint32_t* entry) {
Handle<SeededNumberDictionary> dictionary =
JSObject::NormalizeElements(object);
elements->set_arguments(*dictionary);
// kMaxUInt32 indicates that a context mapped element got deleted. In this
// case we only normalize the elements (aka. migrate to SLOW_SLOPPY).
if (*entry == kMaxUInt32) return dictionary;
uint32_t length = elements->parameter_map_length();
if (*entry >= length) {
*entry = dictionary->FindEntry(*entry - length) + length;
}
return dictionary;
}
static void SloppyDeleteImpl(Handle<JSObject> obj,
Handle<SloppyArgumentsElements> elements,
uint32_t entry) {
// Always normalize element on deleting an entry.
NormalizeArgumentsElements(obj, elements, &entry);
SlowSloppyArgumentsElementsAccessor::SloppyDeleteImpl(obj, elements, entry);
static void DeleteFromArguments(Handle<JSObject> obj, uint32_t entry) {
Handle<FixedArray> arguments =
GetArguments(obj->GetIsolate(), obj->elements());
FastHoleyObjectElementsAccessor::DeleteCommon(obj, entry, arguments);
}
static void AddImpl(Handle<JSObject> object, uint32_t index,
@ -3818,10 +3790,14 @@ class FastSloppyArgumentsElementsAccessor
Handle<FixedArrayBase> store, uint32_t entry,
Handle<Object> value,
PropertyAttributes attributes) {
DCHECK_EQ(object->elements(), *store);
Handle<SloppyArgumentsElements> elements(
SloppyArgumentsElements::cast(*store));
NormalizeArgumentsElements(object, elements, &entry);
Handle<SeededNumberDictionary> dictionary =
JSObject::NormalizeElements(object);
SloppyArgumentsElements* elements = SloppyArgumentsElements::cast(*store);
elements->set_arguments(*dictionary);
uint32_t length = elements->parameter_map_length();
if (entry >= length) {
entry = dictionary->FindEntry(entry - length) + length;
}
SlowSloppyArgumentsElementsAccessor::ReconfigureImpl(object, store, entry,
value, attributes);
}

View File

@ -476,12 +476,12 @@ void JSArgumentsObject::JSArgumentsObjectVerify() {
void JSSloppyArgumentsObject::JSSloppyArgumentsObjectVerify() {
Isolate* isolate = GetIsolate();
if (!map()->is_dictionary_map()) VerifyObjectField(kCalleeOffset);
if (isolate->IsInAnyContext(map(), Context::SLOPPY_ARGUMENTS_MAP_INDEX) ||
isolate->IsInAnyContext(map(),
Context::SLOW_ALIASED_ARGUMENTS_MAP_INDEX) ||
isolate->IsInAnyContext(map(),
Context::FAST_ALIASED_ARGUMENTS_MAP_INDEX)) {
// We can only verify the in-object fields for the original maps.
VerifyObjectField(kLengthOffset);
VerifyObjectField(kCalleeOffset);
}
@ -500,7 +500,6 @@ void SloppyArgumentsElements::SloppyArgumentsElementsVerify(
if (get(kArgumentsIndex)->IsUndefined(isolate)) return;
ElementsKind kind = holder->GetElementsKind();
bool is_fast = kind == FAST_SLOPPY_ARGUMENTS_ELEMENTS;
CHECK(IsFixedArray());
CHECK_GE(length(), 2);
CHECK_EQ(map(), isolate->heap()->sloppy_arguments_elements_map());
@ -515,7 +514,7 @@ void SloppyArgumentsElements::SloppyArgumentsElementsVerify(
CHECK_LE(nofMappedParameters, context_object->length());
CHECK_LE(nofMappedParameters, arg_elements->length());
ElementsAccessor* accessor;
if (is_fast) {
if (kind == FAST_SLOPPY_ARGUMENTS_ELEMENTS) {
accessor = ElementsAccessor::ForKind(FAST_HOLEY_ELEMENTS);
} else {
accessor = ElementsAccessor::ForKind(DICTIONARY_ELEMENTS);
@ -524,18 +523,12 @@ void SloppyArgumentsElements::SloppyArgumentsElementsVerify(
// Verify that each context-mapped argument is either the hole or a valid
// Smi within context length range.
Object* mapped = get_mapped_entry(i);
if (mapped->IsTheHole(isolate)) {
// Slow sloppy arguments can be holey.
if (!is_fast) continue;
// Fast sloppy arguments elements are never holey. Either the element is
// context-mapped or present in the arguments elements.
CHECK(accessor->HasElement(holder, i, arg_elements));
continue;
}
if (mapped->IsTheHole(isolate)) continue;
Object* value = context_object->get(Smi::cast(mapped)->value());
CHECK(value->IsObject());
// None of the context-mapped entries should exist in the arguments
// elements.
// elements unless they have been deleted and readded, which would leave
// the_hole in the parameter map.
CHECK(!accessor->HasElement(holder, i, arg_elements));
}
}

View File

@ -5826,13 +5826,14 @@ Handle<SeededNumberDictionary> JSObject::NormalizeElements(
Handle<JSObject> object) {
DCHECK(!object->HasFixedTypedArrayElements());
Isolate* isolate = object->GetIsolate();
bool is_sloppy_arguments = object->HasSloppyArgumentsElements();
bool is_arguments = object->HasSloppyArgumentsElements();
{
DisallowHeapAllocation no_gc;
FixedArrayBase* elements = object->elements();
if (is_sloppy_arguments) {
elements = SloppyArgumentsElements::cast(elements)->arguments();
if (is_arguments) {
FixedArray* parameter_map = FixedArray::cast(elements);
elements = FixedArrayBase::cast(parameter_map->get(1));
}
if (elements->IsDictionary()) {
@ -5849,7 +5850,7 @@ Handle<SeededNumberDictionary> JSObject::NormalizeElements(
object->GetElementsAccessor()->Normalize(object);
// Switch to using the dictionary as the backing storage for elements.
ElementsKind target_kind = is_sloppy_arguments
ElementsKind target_kind = is_arguments
? SLOW_SLOPPY_ARGUMENTS_ELEMENTS
: object->HasFastStringWrapperElements()
? SLOW_STRING_WRAPPER_ELEMENTS
@ -5858,9 +5859,8 @@ Handle<SeededNumberDictionary> JSObject::NormalizeElements(
// Set the new map first to satify the elements type assert in set_elements().
JSObject::MigrateToMap(object, new_map);
if (is_sloppy_arguments) {
SloppyArgumentsElements::cast(object->elements())
->set_arguments(*dictionary);
if (is_arguments) {
FixedArray::cast(object->elements())->set(1, *dictionary);
} else {
object->set_elements(*dictionary);
}
@ -15597,8 +15597,6 @@ static bool ShouldConvertToFastElements(JSObject* object,
Object* length = JSArray::cast(object)->length();
if (!length->IsSmi()) return false;
*new_capacity = static_cast<uint32_t>(Smi::cast(length)->value());
} else if (object->IsJSSloppyArgumentsObject()) {
return false;
} else {
*new_capacity = dictionary->max_number_key() + 1;
}
@ -15643,7 +15641,7 @@ Maybe<bool> JSObject::AddDataElement(Handle<JSObject> object, uint32_t index,
FixedArrayBase* elements = object->elements();
ElementsKind dictionary_kind = DICTIONARY_ELEMENTS;
if (IsSloppyArgumentsElementsKind(kind)) {
elements = SloppyArgumentsElements::cast(elements)->arguments();
elements = FixedArrayBase::cast(FixedArray::cast(elements)->get(1));
dictionary_kind = SLOW_SLOPPY_ARGUMENTS_ELEMENTS;
} else if (IsStringWrapperElementsKind(kind)) {
dictionary_kind = SLOW_STRING_WRAPPER_ELEMENTS;
@ -15656,7 +15654,7 @@ Maybe<bool> JSObject::AddDataElement(Handle<JSObject> object, uint32_t index,
SeededNumberDictionary::cast(elements),
index, &new_capacity)
? BestFittingFastElementsKind(*object)
: dictionary_kind;
: dictionary_kind; // Overwrite in case of arguments.
} else if (ShouldConvertToSlowElements(
*object, static_cast<uint32_t>(elements->length()), index,
&new_capacity)) {
@ -20645,5 +20643,6 @@ ElementsKind JSArrayIterator::ElementsKindForInstanceType(InstanceType type) {
return kind;
}
}
} // namespace internal
} // namespace v8

View File

@ -2707,7 +2707,7 @@ class JSSloppyArgumentsObject: public JSArgumentsObject {
static const int kCalleeOffset = JSArgumentsObject::kHeaderSize;
static const int kSize = kCalleeOffset + kPointerSize;
// Indices of in-object properties.
static const int kCalleeIndex = kLengthIndex + 1;
static const int kCalleeIndex = 1;
DECL_ACCESSORS(callee, Object)

View File

@ -248,93 +248,26 @@ assertEquals(117, arg_set(0xFFFFFFFF));
return arguments
};
var args = f(1, 2);
%HeapObjectVerify(args);
assertEquals(1, args[0]);
assertEquals(2, args[1]);
assertEquals(key, args[key]);
assertEquals(2, args.length);
delete args[0];
%HeapObjectVerify(args);
assertEquals(undefined, args[0]);
assertEquals(2, args[1]);
assertEquals(key, args[key]);
assertEquals(2, args.length);
delete args[1];
%HeapObjectVerify(args);
assertEquals(undefined, args[0]);
assertEquals(undefined, args[1]);
assertEquals(key, args[key]);
assertEquals(2, args.length);
delete args[key];
%HeapObjectVerify(args);
assertEquals(undefined, args[0]);
assertEquals(undefined, args[1]);
assertEquals(undefined, args[key]);
assertEquals(2, args.length);
})();
(function testDeleteSlowSloppyArguments2() {
function f(a) {
return arguments
};
var args = f(1, 2);
%HeapObjectVerify(args);
assertEquals(1, args[0]);
assertEquals(2, args[1]);
assertEquals(2, args.length);
delete args[1];
%HeapObjectVerify(args);
assertEquals(1, args[0]);
assertEquals(undefined, args[1]);
assertEquals(undefined, args[2]);
assertEquals(2, args.length);
delete args[0];
%HeapObjectVerify(args);
assertEquals(undefined, args[0]);
assertEquals(undefined, args[1]);
assertEquals(undefined, args[2]);
assertEquals(2, args.length);
})();
(function testSloppyArgumentProperties() {
function f(a, b) { return arguments }
let args = f(1, 2, 3, 4);
%HeapObjectVerify(args);
assertEquals(4, args.length);
args.foo = "foo";
%HeapObjectVerify(args);
assertEquals("foo", args.foo);
assertEquals(4, args.length);
delete args.foo;
%HeapObjectVerify(args);
assertEquals(undefined, args.foo);
assertEquals(4, args.length);
})();
(function testSloppyArgumentsLengthMapChange() {
function f(a) { return arguments };
let args1 = f(1);
let args2 = f(1,2);
assertTrue(%HaveSameMap(args1, args2));
args2.length = 12;
assertTrue(%HaveSameMap(args1, args2));
args2.length = "aa"
assertTrue(%HaveSameMap(args1, args2));
let args3 = f(1);
let args4 = f(1,2);
// Creating holes causes map transitions.
assertTrue(%HaveSameMap(args1, args3));
assertTrue(%HaveSameMap(args1, args4));
delete args3[0];
assertFalse(%HaveSameMap(args1, args3));
delete args4[1];
assertFalse(%HaveSameMap(args1, args4));
})();

View File

@ -25,10 +25,7 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
// Flags: --allow-natives-syntax
// Check that slicing array of holes keeps it as array of holes
(function() {
var array = new Array(10);
for (var i = 0; i < 7; i++) {
@ -225,9 +222,7 @@
// Check slicing on arguments object.
(function() {
function func(expected, a0, a1, a2) {
let result = Array.prototype.slice.call(arguments, 1);
assertEquals(expected, result);
%HeapObjectVerify(result);
assertEquals(expected, Array.prototype.slice.call(arguments, 1));
}
func([]);
@ -245,9 +240,7 @@
assertEquals(undefined, y);
y = 239;
assertEquals(1, arguments.length); // arguments length is the same.
let result = Array.prototype.slice.call(arguments, 0);
assertEquals([x], result);
%HeapObjectVerify(result);
assertEquals([x], Array.prototype.slice.call(arguments, 0));
}
func('a');
@ -258,10 +251,7 @@
function func(x, y) {
assertEquals(1, arguments.length);
arguments.length = 7;
let result = Array.prototype.slice.call(arguments, 0);
assertEquals([x,,,,,,,], result);
%HeapObjectVerify(result);
%HeapObjectVerify(arguments);
assertEquals([x,,,,,,,], Array.prototype.slice.call(arguments, 0));
}
func('a');
@ -273,10 +263,7 @@
function func(x, y) {
assertEquals(1, arguments.length);
arguments.length = 'foobar';
let result = Array.prototype.slice.call(arguments, 0);
assertEquals([], result);
%HeapObjectVerify(result);
%HeapObjectVerify(arguments);
assertEquals([], Array.prototype.slice.call(arguments, 0));
}
func('a');
@ -288,9 +275,7 @@
function func(x, y) {
assertEquals(1, arguments.length);
arguments[3] = 239;
let result = Array.prototype.slice.call(arguments, 0);
assertEquals([x], result);
%HeapObjectVerify(result);
assertEquals([x], Array.prototype.slice.call(arguments, 0));
}
func('a');
@ -301,9 +286,7 @@
function func(x, y, z) {
assertEquals(3, arguments.length);
delete arguments[1];
let result = Array.prototype.slice.call(arguments, 0);
assertEquals([x,,z], result);
%HeapObjectVerify(result);
assertEquals([x,,z], Array.prototype.slice.call(arguments, 0));
}
func('a', 'b', 'c');
@ -317,7 +300,6 @@
var result = Array.prototype.slice.call(arguments);
delete arguments.__proto__[1];
assertEquals([1,5,3], result);
%HeapObjectVerify(result);
}
f(1,2,3);
})();

View File

@ -27,14 +27,10 @@
// Execises ArgumentsAccessStub::GenerateNewNonStrictSlow.
// Flags: --allow-natives-syntax
function f(a, a) {
assertEquals(2, a);
assertEquals(1, arguments[0]);
assertEquals(2, arguments[1]);
assertEquals(2, arguments.length);
%HeapObjectVerify(arguments);
}
f(1, 2);

View File

@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax
function enumerate(o) {
var keys = [];
for (var key in o) keys.push(key);
@ -12,13 +10,11 @@ function enumerate(o) {
(function testSlowSloppyArgumentsElements() {
function slowSloppyArguments(a, b, c) {
%HeapObjectVerify(arguments);
arguments[10000] = "last";
arguments[4000] = "first";
arguments[6000] = "second";
arguments[5999] = "x";
arguments[3999] = "y";
%HeapObjectVerify(arguments);
return arguments;
}
assertEquals(["0", "1", "2", "3999", "4000", "5999", "6000", "10000"],
@ -33,12 +29,10 @@ function enumerate(o) {
Object.defineProperty(arguments, 10000, {
enumerable: false, configurable: false, value: "NOPE"
});
%HeapObjectVerify(arguments);
arguments[4000] = "first";
arguments[6000] = "second";
arguments[5999] = "x";
arguments[3999] = "y";
%HeapObjectVerify(arguments);
return arguments;
}
@ -49,13 +43,11 @@ function enumerate(o) {
enumerate(slowSloppyArguments(1,2,3)));
})();
(function testFastSloppyArgumentsElements() {
function fastSloppyArguments(a, b, c) {
arguments[5] = 1;
arguments[7] = 0;
arguments[3] = 2;
%HeapObjectVerify(arguments);
return arguments;
}
assertEquals(["0", "1", "2", "3", "5", "7"],
@ -66,11 +58,7 @@ function enumerate(o) {
function fastSloppyArguments2(a, b, c) {
delete arguments[0];
%DebugPrint(arguments);
%HeapObjectVerify(arguments);
arguments[0] = "test";
%DebugPrint(arguments);
%HeapObjectVerify(arguments);
return arguments;
}
@ -83,10 +71,8 @@ function enumerate(o) {
Object.defineProperty(arguments, 5, {
enumerable: false, configurable: false, value: "NOPE"
});
%HeapObjectVerify(arguments);
arguments[7] = 0;
arguments[3] = 2;
%HeapObjectVerify(arguments);
return arguments;
}
assertEquals(
@ -97,12 +83,10 @@ function enumerate(o) {
function fastSloppyArguments2(a, b, c) {
delete arguments[0];
%HeapObjectVerify(arguments);
Object.defineProperty(arguments, 1, {
enumerable: false, configurable: false, value: "NOPE"
});
arguments[0] = "test";
%HeapObjectVerify(arguments);
return arguments;
}

View File

@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --expose-gc --allow-natives-syntax
// Flags: --expose-gc
//
function getRandomProperty(v, rand) {
var properties = Object.getOwnPropertyNames(v);
if ("constructor" && v.constructor.hasOwnProperty()) {; }
@ -11,12 +11,10 @@ function getRandomProperty(v, rand) {
return properties[rand % properties.length];
}
var args = (function( b) { return arguments; })("foo", NaN, "bar");
args.__p_293850326 = "foo";
%HeapObjectVerify(args);
args.__defineGetter__(getRandomProperty( 990787501), function() {
var __v_18 = (function( b) { return arguments; })("foo", NaN, "bar");
__v_18.__p_293850326 = "foo";
__v_18.__defineGetter__(getRandomProperty( 990787501), function() {
gc();
return args.__p_293850326;
return __v_18.__p_293850326;
});
%HeapObjectVerify(args);
Array.prototype.indexOf.call(args)
Array.prototype.indexOf.call(__v_18)