AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

[builtins] Skip redundantly setting array length in Array.from

Browse Source 
Now that Array.from() always calls the runtime to set the length if it's
not equal to the current length, don't actually set it on the fast path
since it's unobservable and doesn't change anything.

Also remove check for the array being writable since it's no longer
needed.

Change-Id: I0928d80b445807912fd925f7957c9a76385fc6bc
Reviewed-on: https://chromium-review.googlesource.com/961403
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51929}
This commit is contained in:
Dan Elphick 2018-03-14 13:40:21 +00:00 committed by Commit Bot
parent 4dc6889753
commit 7f44053c9e
1 changed files with 9 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/builtins/builtins-array-gen.cc
View File

@ -1945,13 +1945,10 @@ class ArrayPopulatorAssembler : public CodeStubAssembler {
void GenerateSetLength(TNode<Context> context, TNode<Object> array,
TNode<Number> length) {
Label fast(this), runtime(this), done(this);
// TODO(delphick): We should be able to skip the fast set altogether, if the
// length already equals the expected length, which it always is now on the
// fast path.
// Only set the length in this stub if
// 1) the array has fast elements,
// 2) the length is writable,
// 3) the new length is equal to the old length.
// There's no need to set the length, if
// 1) the array is a fast JS array and
// 2) the new length is equal to the old length.
// as the set is not observable. Otherwise fall back to the run-time.
// 1) Check that the array has fast elements.
// TODO(delphick): Consider changing this since it does an an unnecessary
@ -1968,20 +1965,11 @@ class ArrayPopulatorAssembler : public CodeStubAssembler {
TNode<Smi> old_length = LoadFastJSArrayLength(fast_array);
CSA_ASSERT(this, TaggedIsPositiveSmi(old_length));
// 2) Ensure that the length is writable.
// TODO(delphick): This check may be redundant due to the
// BranchIfFastJSArray above.
EnsureArrayLengthWritable(LoadMap(fast_array), &runtime);
// 3) If the created array's length does not match the required length,
// then use the runtime to set the property as that will insert holes
// into excess elements or shrink the backing store as appropriate.
GotoIf(SmiNotEqual(length_smi, old_length), &runtime);
StoreObjectFieldNoWriteBarrier(fast_array, JSArray::kLengthOffset,
length_smi);
Goto(&done);
// 2) If the created array's length matches the required length, then
// there's nothing else to do. Otherwise use the runtime to set the
// property as that will insert holes into excess elements or shrink
// the backing store as appropriate.
Branch(SmiNotEqual(length_smi, old_length), &runtime, &done);
}
BIND(&runtime);