Commit ac3f98d5 authored by Caitlin Potter's avatar Caitlin Potter Committed by Commit Bot

[builtins] put SetPropertyInLiteral in a code-stub

There are several core changes in this stub:

1) add a version of KeyedStoreGenericGenerator::SetPropertyInLiteral()
which supports indexed properties directly, witthout KeyedStore

2) add a code stub for SetPropertyInLiteral which uses the version
supporting indexed properties

3) Use the code stub in CloneObjectIC, rather than using the smaller
special-cased version which does not handle Names.

Item 1) involves a refactoring which adds a nice way to reuse code in
KeyedStoreGenericAssembler, which allows deleting a bunch of copy/pasted
code. This makes it easy to reuse the index handling in
KeyedStoreGeneric() without adding adding a bunch more duplicated
handling. Because of this, I consider this to be somewhat of a cleanup,
though if the copied code is preferred, I'm happy to revert to that.

Item 2) is needed for Object.fromEntries(), as it's better to not
require falling back to the slow path if a key happens to be an Smi ---
but this is also optional.

Item 3) benefits the codebase by allowing Object.fromEntries() to use
this fast path without calling into the runtime, and without duplicating
code which is also used by CloneObjectIC.

I am skeptical that this should affect performance significantly. I've
run ObjectLiteralSpread tests, and the mean of scores over 100 runs is
somewhat surprising: CloneObjectIC --- the only user of this code,
has an increased average score, while the polyfill cases score slightly
worse --- However, the overall changes are small and likely flukes.
The complete processed test output is below:

```
// Mean of 100 runs of each benchmark

Babel-ObjectLiteralSpread:
-----+---------------------------+---------------------------+-------
     | With patch                | Without patch             | diff
Mean | 11530.87                  | 12142.92                  | -5.04%
-----+---------------------------+---------------------------+-------

BabelAndOverwrite-ObjectLiteralSpread:
-----+---------------------------+---------------------------+-------
     | With patch                | Without patch             | diff
Mean | 10881.41                  | 11260.81                  | -3.37%
-----+---------------------------+---------------------------+-------

ObjectAssign-ObjectLiteralSpread:
-----+---------------------------+---------------------------+-------
     | With patch                | Without patch             | diff
Mean | 6188.92                   | 6358.55                   | -2.67%
-----+---------------------------+---------------------------+-------

ObjectAssignAndOverwrite-ObjectLiteralSpread:
-----+---------------------------+---------------------------+-------
     | With patch                | Without patch             | diff
Mean | 6112.80                   | 6275.54                   | -1.61%
-----+---------------------------+---------------------------+-------

ObjectSpread-ObjectLiteralSpread:
-----+---------------------------+---------------------------+-------
     | With patch                | Without patch             | diff
Mean | 51942.93                  | 50713.17                  | +3.46%
-----+---------------------------+---------------------------+-------

ObjectSpreadAndOverwrite-ObjectLiteralSpread:
-----+---------------------------+---------------------------+-------
     | With patch                | Without patch             | diff
Mean | 51375.23                  | 50833.29                  | +2.09%
-----+---------------------------+---------------------------+-------
```

BUG=v8:8238, v8:8021
R=ishell@chromium.org, jkummerow@chromium.org

Change-Id: I43e102fc461ffd389b5d6810a73f86e5012d7dee
Reviewed-on: https://chromium-review.googlesource.com/c/1277751
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56957}
parent 395078d7
...@@ -1327,6 +1327,7 @@ namespace internal { ...@@ -1327,6 +1327,7 @@ namespace internal {
ASM(DoubleToI) \ ASM(DoubleToI) \
TFC(GetProperty, GetProperty, 1) \ TFC(GetProperty, GetProperty, 1) \
TFS(SetProperty, kReceiver, kKey, kValue) \ TFS(SetProperty, kReceiver, kKey, kValue) \
TFS(SetPropertyInLiteral, kReceiver, kKey, kValue) \
ASM(MathPowInternal) \ ASM(MathPowInternal) \
\ \
/* Trace */ \ /* Trace */ \
......
...@@ -1296,5 +1296,19 @@ TF_BUILTIN(SetProperty, CodeStubAssembler) { ...@@ -1296,5 +1296,19 @@ TF_BUILTIN(SetProperty, CodeStubAssembler) {
value, LanguageMode::kStrict); value, LanguageMode::kStrict);
} }
// ES6 CreateDataProperty(), specialized for the case where objects are still
// being initialized, and have not yet been made accessible to the user. Thus,
// any operation here should be unobservable until after the object has been
// returned.
TF_BUILTIN(SetPropertyInLiteral, CodeStubAssembler) {
TNode<Context> context = CAST(Parameter(Descriptor::kContext));
TNode<JSObject> receiver = CAST(Parameter(Descriptor::kReceiver));
TNode<Object> key = CAST(Parameter(Descriptor::kKey));
TNode<Object> value = CAST(Parameter(Descriptor::kValue));
KeyedStoreGenericGenerator::SetPropertyInLiteral(state(), context, receiver,
key, value);
}
} // namespace internal } // namespace internal
} // namespace v8 } // namespace v8
...@@ -2603,6 +2603,13 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler { ...@@ -2603,6 +2603,13 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
return CallBuiltin(Builtins::kSetProperty, context, receiver, key, value); return CallBuiltin(Builtins::kSetProperty, context, receiver, key, value);
} }
TNode<Object> SetPropertyInLiteral(TNode<Context> context,
TNode<JSObject> receiver,
TNode<Object> key, TNode<Object> value) {
return CallBuiltin(Builtins::kSetPropertyInLiteral, context, receiver, key,
value);
}
Node* GetMethod(Node* context, Node* object, Handle<Name> name, Node* GetMethod(Node* context, Node* object, Handle<Name> name,
Label* if_null_or_undefined); Label* if_null_or_undefined);
......
...@@ -3546,13 +3546,12 @@ void AccessorAssembler::GenerateCloneObjectIC_Slow() { ...@@ -3546,13 +3546,12 @@ void AccessorAssembler::GenerateCloneObjectIC_Slow() {
GotoIfNot(IsEmptyFixedArray(LoadElements(CAST(source))), &call_runtime); GotoIfNot(IsEmptyFixedArray(LoadElements(CAST(source))), &call_runtime);
ForEachEnumerableOwnProperty( ForEachEnumerableOwnProperty(context, map, CAST(source),
context, map, CAST(source), [=](TNode<Name> key, TNode<Object> value) {
[=](TNode<Name> key, TNode<Object> value) { SetPropertyInLiteral(context, result, key,
KeyedStoreGenericGenerator::SetPropertyInLiteral(state(), context, value);
result, key, value); },
}, &call_runtime);
&call_runtime);
Goto(&done); Goto(&done);
BIND(&call_runtime); BIND(&call_runtime);
......
This diff is collapsed.
...@@ -33,7 +33,7 @@ class KeyedStoreGenericGenerator { ...@@ -33,7 +33,7 @@ class KeyedStoreGenericGenerator {
static void SetPropertyInLiteral(compiler::CodeAssemblerState* state, static void SetPropertyInLiteral(compiler::CodeAssemblerState* state,
TNode<Context> context, TNode<Context> context,
TNode<JSObject> receiver, TNode<Name> key, TNode<JSObject> receiver, TNode<Object> key,
TNode<Object> value); TNode<Object> value);
}; };
......
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