Skip to content

Commit 03e9d41

Browse files
brnCommit Bot
brn
authored and
Commit Bot
committed
Reland: Reimplement Object.entries/values as CSA to optimize performance.
Add Object.entries/values builtins to debug-evaluate.cc whitelist macro. This fix revert commit of https://chromium-review.googlesource.com/c/v8/v8/+/859937 Original is https://chromium-review.googlesource.com/c/v8/v8/+/810504 >> Reimplements Object.entries/values as CSA to optimize performance. See more detail about https://bugs.chromium.org/p/v8/issues/ Issue 6804. This reverts commit 1b49f72. Bug: v8:6804 Change-Id: I57e8b66e1c4ece2abb52e1630a97fbfd4070d810 Reviewed-on: https://chromium-review.googlesource.com/860679 Commit-Queue: Yang Guo <[email protected]> Reviewed-by: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#50492}
1 parent f9feb5f commit 03e9d41

12 files changed

Lines changed: 420 additions & 67 deletions

‎AUTHORS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ Sanjoy Das <[email protected]>
135135
Seo Sanghyeon <[email protected]>
136136
Stefan Penner <[email protected]>
137137
Sylvestre Ledru <[email protected]>
138+
Taketoshi Aono <[email protected]>
138139
Tiancheng "Timothy" Gu <[email protected]>
139140
Tobias Burnus <[email protected]>
140141
Victor Costan <[email protected]>

‎src/bootstrapper.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,9 +1508,9 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
15081508
object_function, "keys", Builtins::kObjectKeys, 1, true);
15091509
native_context()->set_object_keys(*object_keys);
15101510
SimpleInstallFunction(object_function, factory->entries_string(),
1511-
Builtins::kObjectEntries, 1, false);
1511+
Builtins::kObjectEntries, 1, true);
15121512
SimpleInstallFunction(object_function, factory->values_string(),
1513-
Builtins::kObjectValues, 1, false);
1513+
Builtins::kObjectValues, 1, true);
15141514

15151515
SimpleInstallFunction(isolate->initial_object_prototype(),
15161516
"__defineGetter__", Builtins::kObjectDefineGetter, 2,

‎src/builtins/builtins-definitions.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -754,7 +754,7 @@ namespace internal {
754754
CPP(ObjectDefineProperties) \
755755
CPP(ObjectDefineProperty) \
756756
CPP(ObjectDefineSetter) \
757-
CPP(ObjectEntries) \
757+
TFJ(ObjectEntries, 1, kObject) \
758758
CPP(ObjectFreeze) \
759759
TFJ(ObjectGetOwnPropertyDescriptor, \
760760
SharedFunctionInfo::kDontAdaptArgumentsSentinel) \
@@ -784,7 +784,7 @@ namespace internal {
784784
/* ES #sec-object.prototype.tolocalestring */ \
785785
TFJ(ObjectPrototypeToLocaleString, 0) \
786786
CPP(ObjectSeal) \
787-
CPP(ObjectValues) \
787+
TFJ(ObjectValues, 1, kObject) \
788788
\
789789
/* instanceof */ \
790790
TFC(OrdinaryHasInstance, Compare, 1) \

‎src/builtins/builtins-object-gen.cc‎

Lines changed: 301 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ namespace internal {
1616
// ES6 section 19.1 Object Objects
1717

1818
typedef compiler::Node Node;
19+
template <class T>
20+
using TNode = CodeStubAssembler::TNode<T>;
1921

2022
class ObjectBuiltinsAssembler : public CodeStubAssembler {
2123
public:
@@ -34,6 +36,46 @@ class ObjectBuiltinsAssembler : public CodeStubAssembler {
3436
Node* ConstructDataDescriptor(Node* context, Node* value, Node* writable,
3537
Node* enumerable, Node* configurable);
3638
Node* GetAccessorOrUndefined(Node* accessor, Label* if_bailout);
39+
40+
Node* IsSpecialReceiverMap(SloppyTNode<Map> map);
41+
};
42+
43+
class ObjectEntriesValuesBuiltinsAssembler : public ObjectBuiltinsAssembler {
44+
public:
45+
explicit ObjectEntriesValuesBuiltinsAssembler(
46+
compiler::CodeAssemblerState* state)
47+
: ObjectBuiltinsAssembler(state) {}
48+
49+
protected:
50+
enum CollectType { kEntries, kValues };
51+
52+
TNode<Word32T> IsStringWrapperElementsKind(TNode<Map> map);
53+
54+
TNode<BoolT> IsPropertyEnumerable(TNode<Uint32T> details);
55+
56+
TNode<BoolT> IsPropertyKindAccessor(TNode<Uint32T> kind);
57+
58+
TNode<BoolT> IsPropertyKindData(TNode<Uint32T> kind);
59+
60+
TNode<Uint32T> HasHiddenPrototype(TNode<Map> map);
61+
62+
TNode<Uint32T> LoadPropertyKind(TNode<Uint32T> details) {
63+
return DecodeWord32<PropertyDetails::KindField>(details);
64+
}
65+
66+
void GetOwnValuesOrEntries(TNode<Context> context, TNode<Object> maybe_object,
67+
CollectType collect_type);
68+
69+
void GotoIfMapHasSlowProperties(TNode<Map> map, Label* if_slow);
70+
71+
TNode<JSArray> FastGetOwnValuesOrEntries(
72+
TNode<Context> context, TNode<JSObject> object,
73+
Label* if_call_runtime_with_fast_path, Label* if_no_properties,
74+
CollectType collect_type);
75+
76+
TNode<JSArray> FinalizeValuesOrEntriesJSArray(
77+
TNode<Context> context, TNode<FixedArray> values_or_entries,
78+
TNode<IntPtrT> size, TNode<Map> array_map, Label* if_empty);
3779
};
3880

3981
void ObjectBuiltinsAssembler::ReturnToStringFormat(Node* context,
@@ -97,6 +139,249 @@ Node* ObjectBuiltinsAssembler::ConstructDataDescriptor(Node* context,
97139
return js_desc;
98140
}
99141

142+
Node* ObjectBuiltinsAssembler::IsSpecialReceiverMap(SloppyTNode<Map> map) {
143+
CSA_SLOW_ASSERT(this, IsMap(map));
144+
Node* is_special = IsSpecialReceiverInstanceType(LoadMapInstanceType(map));
145+
uint32_t mask =
146+
Map::HasNamedInterceptorBit::kMask | Map::IsAccessCheckNeededBit::kMask;
147+
USE(mask);
148+
// Interceptors or access checks imply special receiver.
149+
CSA_ASSERT(this,
150+
SelectConstant(IsSetWord32(LoadMapBitField(map), mask), is_special,
151+
Int32Constant(1), MachineRepresentation::kWord32));
152+
return is_special;
153+
}
154+
155+
TNode<Word32T>
156+
ObjectEntriesValuesBuiltinsAssembler::IsStringWrapperElementsKind(
157+
TNode<Map> map) {
158+
Node* kind = LoadMapElementsKind(map);
159+
return Word32Or(
160+
Word32Equal(kind, Int32Constant(FAST_STRING_WRAPPER_ELEMENTS)),
161+
Word32Equal(kind, Int32Constant(SLOW_STRING_WRAPPER_ELEMENTS)));
162+
}
163+
164+
TNode<BoolT> ObjectEntriesValuesBuiltinsAssembler::IsPropertyEnumerable(
165+
TNode<Uint32T> details) {
166+
TNode<Uint32T> attributes =
167+
DecodeWord32<PropertyDetails::AttributesField>(details);
168+
return IsNotSetWord32(attributes, PropertyAttributes::DONT_ENUM);
169+
}
170+
171+
TNode<BoolT> ObjectEntriesValuesBuiltinsAssembler::IsPropertyKindAccessor(
172+
TNode<Uint32T> kind) {
173+
return Word32Equal(kind, Int32Constant(PropertyKind::kAccessor));
174+
}
175+
176+
TNode<BoolT> ObjectEntriesValuesBuiltinsAssembler::IsPropertyKindData(
177+
TNode<Uint32T> kind) {
178+
return Word32Equal(kind, Int32Constant(PropertyKind::kData));
179+
}
180+
181+
TNode<Uint32T> ObjectEntriesValuesBuiltinsAssembler::HasHiddenPrototype(
182+
TNode<Map> map) {
183+
TNode<Uint32T> bit_field3 = LoadMapBitField3(map);
184+
return DecodeWord32<Map::HasHiddenPrototypeBit>(bit_field3);
185+
}
186+
187+
void ObjectEntriesValuesBuiltinsAssembler::GetOwnValuesOrEntries(
188+
TNode<Context> context, TNode<Object> maybe_object,
189+
CollectType collect_type) {
190+
TNode<JSObject> object = TNode<JSObject>::UncheckedCast(
191+
CallBuiltin(Builtins::kToObject, context, maybe_object));
192+
193+
Label if_call_runtime_with_fast_path(this, Label::kDeferred),
194+
if_call_runtime(this, Label::kDeferred),
195+
if_no_properties(this, Label::kDeferred);
196+
197+
TNode<Map> map = LoadMap(object);
198+
GotoIfNot(IsJSObjectMap(map), &if_call_runtime);
199+
GotoIfMapHasSlowProperties(map, &if_call_runtime);
200+
201+
TNode<FixedArrayBase> elements = LoadElements(object);
202+
// If the object has elements, we treat it as slow case.
203+
// So, we go to runtime call.
204+
GotoIfNot(IsEmptyFixedArray(elements), &if_call_runtime_with_fast_path);
205+
206+
TNode<JSArray> result = FastGetOwnValuesOrEntries(
207+
context, object, &if_call_runtime_with_fast_path, &if_no_properties,
208+
collect_type);
209+
Return(result);
210+
211+
BIND(&if_no_properties);
212+
{
213+
Node* native_context = LoadNativeContext(context);
214+
Node* array_map = LoadJSArrayElementsMap(PACKED_ELEMENTS, native_context);
215+
Node* empty_array = AllocateJSArray(PACKED_ELEMENTS, array_map,
216+
IntPtrConstant(0), SmiConstant(0));
217+
Return(empty_array);
218+
}
219+
220+
BIND(&if_call_runtime_with_fast_path);
221+
{
222+
// In slow case, we simply call runtime.
223+
if (collect_type == CollectType::kEntries) {
224+
Return(CallRuntime(Runtime::kObjectEntries, context, object));
225+
} else {
226+
DCHECK(collect_type == CollectType::kValues);
227+
Return(CallRuntime(Runtime::kObjectValues, context, object));
228+
}
229+
}
230+
231+
BIND(&if_call_runtime);
232+
{
233+
// In slow case, we simply call runtime.
234+
if (collect_type == CollectType::kEntries) {
235+
Return(CallRuntime(Runtime::kObjectEntriesSkipFastPath, context, object));
236+
} else {
237+
DCHECK(collect_type == CollectType::kValues);
238+
Return(CallRuntime(Runtime::kObjectValuesSkipFastPath, context, object));
239+
}
240+
}
241+
}
242+
243+
void ObjectEntriesValuesBuiltinsAssembler::GotoIfMapHasSlowProperties(
244+
TNode<Map> map, Label* if_slow) {
245+
GotoIf(IsStringWrapperElementsKind(map), if_slow);
246+
GotoIf(IsSpecialReceiverMap(map), if_slow);
247+
GotoIf(HasHiddenPrototype(map), if_slow);
248+
GotoIf(IsDictionaryMap(map), if_slow);
249+
}
250+
251+
TNode<JSArray> ObjectEntriesValuesBuiltinsAssembler::FastGetOwnValuesOrEntries(
252+
TNode<Context> context, TNode<JSObject> object,
253+
Label* if_call_runtime_with_fast_path, Label* if_no_properties,
254+
CollectType collect_type) {
255+
Node* native_context = LoadNativeContext(context);
256+
TNode<Map> array_map =
257+
LoadJSArrayElementsMap(PACKED_ELEMENTS, native_context);
258+
TNode<Map> map = LoadMap(object);
259+
TNode<Uint32T> bit_field3 = LoadMapBitField3(map);
260+
261+
Label if_has_enum_cache(this), if_not_has_enum_cache(this),
262+
collect_entries(this);
263+
Node* object_enum_length =
264+
DecodeWordFromWord32<Map::EnumLengthBits>(bit_field3);
265+
Node* has_enum_cache = WordNotEqual(
266+
object_enum_length, IntPtrConstant(kInvalidEnumCacheSentinel));
267+
268+
// In case, we found enum_cache in object,
269+
// we use it as array_length becuase it has same size for
270+
// Object.(entries/values) result array object length.
271+
// So object_enum_length use less memory space than
272+
// NumberOfOwnDescriptorsBits value.
273+
// And in case, if enum_cache_not_found,
274+
// we call runtime and initialize enum_cache for subsequent call of
275+
// CSA fast path.
276+
Branch(has_enum_cache, &if_has_enum_cache, if_call_runtime_with_fast_path);
277+
278+
BIND(&if_has_enum_cache);
279+
{
280+
GotoIf(WordEqual(object_enum_length, IntPtrConstant(0)), if_no_properties);
281+
TNode<FixedArray> values_or_entries = TNode<FixedArray>::UncheckedCast(
282+
AllocateFixedArray(PACKED_ELEMENTS, object_enum_length,
283+
INTPTR_PARAMETERS, kAllowLargeObjectAllocation));
284+
285+
// If in case we have enum_cache,
286+
// we can't detect accessor of object until loop through descritpros.
287+
// So if object might have accessor,
288+
// we will remain invalid addresses of FixedArray.
289+
// Because in that case, we need to jump to runtime call.
290+
// So the array filled by the-hole even if enum_cache exists.
291+
FillFixedArrayWithValue(PACKED_ELEMENTS, values_or_entries,
292+
IntPtrConstant(0), object_enum_length,
293+
Heap::kTheHoleValueRootIndex);
294+
295+
TVARIABLE(IntPtrT, var_result_index, IntPtrConstant(0));
296+
TVARIABLE(IntPtrT, var_descriptor_index, IntPtrConstant(0));
297+
Variable* vars[] = {&var_descriptor_index, &var_result_index};
298+
// Let desc be ? O.[[GetOwnProperty]](key).
299+
TNode<DescriptorArray> descriptors = LoadMapDescriptors(map);
300+
Label loop(this, 2, vars), after_loop(this), loop_condition(this);
301+
Branch(IntPtrEqual(var_descriptor_index, object_enum_length), &after_loop,
302+
&loop);
303+
304+
// We dont use BuildFastLoop.
305+
// Instead, we use hand-written loop
306+
// because of we need to use 'continue' functionality.
307+
BIND(&loop);
308+
{
309+
// Currently, we will not invoke getters,
310+
// so, map will not be changed.
311+
CSA_ASSERT(this, WordEqual(map, LoadMap(object)));
312+
TNode<Uint32T> descriptor_index = TNode<Uint32T>::UncheckedCast(
313+
TruncateWordToWord32(var_descriptor_index));
314+
Node* next_key = DescriptorArrayGetKey(descriptors, descriptor_index);
315+
316+
// Skip Symbols.
317+
GotoIf(IsSymbol(next_key), &loop_condition);
318+
319+
TNode<Uint32T> details = TNode<Uint32T>::UncheckedCast(
320+
DescriptorArrayGetDetails(descriptors, descriptor_index));
321+
TNode<Uint32T> kind = LoadPropertyKind(details);
322+
323+
// If property is accessor, we escape fast path and call runtime.
324+
GotoIf(IsPropertyKindAccessor(kind), if_call_runtime_with_fast_path);
325+
CSA_ASSERT(this, IsPropertyKindData(kind));
326+
327+
// If desc is not undefined and desc.[[Enumerable]] is true, then
328+
GotoIfNot(IsPropertyEnumerable(details), &loop_condition);
329+
330+
VARIABLE(var_property_value, MachineRepresentation::kTagged,
331+
UndefinedConstant());
332+
Node* descriptor_name_index = DescriptorNumberToIndex(descriptor_index);
333+
334+
// Let value be ? Get(O, key).
335+
LoadPropertyFromFastObject(object, map, descriptors,
336+
descriptor_name_index, details,
337+
&var_property_value);
338+
339+
// If kind is "value", append value to properties.
340+
Node* value = var_property_value.value();
341+
342+
if (collect_type == CollectType::kEntries) {
343+
// Let entry be CreateArrayFromList(« key, value »).
344+
Node* array = nullptr;
345+
Node* elements = nullptr;
346+
std::tie(array, elements) = AllocateUninitializedJSArrayWithElements(
347+
PACKED_ELEMENTS, array_map, SmiConstant(2), nullptr,
348+
IntPtrConstant(2));
349+
StoreFixedArrayElement(elements, 0, next_key, SKIP_WRITE_BARRIER);
350+
StoreFixedArrayElement(elements, 1, value, SKIP_WRITE_BARRIER);
351+
value = array;
352+
}
353+
354+
StoreFixedArrayElement(values_or_entries, var_result_index, value);
355+
Increment(&var_result_index, 1);
356+
Goto(&loop_condition);
357+
358+
BIND(&loop_condition);
359+
{
360+
Increment(&var_descriptor_index, 1);
361+
Branch(IntPtrEqual(var_descriptor_index, object_enum_length),
362+
&after_loop, &loop);
363+
}
364+
}
365+
BIND(&after_loop);
366+
return FinalizeValuesOrEntriesJSArray(context, values_or_entries,
367+
var_result_index, array_map,
368+
if_no_properties);
369+
}
370+
}
371+
372+
TNode<JSArray>
373+
ObjectEntriesValuesBuiltinsAssembler::FinalizeValuesOrEntriesJSArray(
374+
TNode<Context> context, TNode<FixedArray> result, TNode<IntPtrT> size,
375+
TNode<Map> array_map, Label* if_empty) {
376+
CSA_ASSERT(this, IsJSArrayMap(array_map));
377+
378+
GotoIf(IntPtrEqual(size, IntPtrConstant(0)), if_empty);
379+
Node* array = AllocateUninitializedJSArrayWithoutElements(
380+
array_map, SmiTag(size), nullptr);
381+
StoreObjectField(array, JSArray::kElementsOffset, result);
382+
return TNode<JSArray>::UncheckedCast(array);
383+
}
384+
100385
TF_BUILTIN(ObjectPrototypeToLocaleString, CodeStubAssembler) {
101386
TNode<Context> context = CAST(Parameter(Descriptor::kContext));
102387
TNode<Object> receiver = CAST(Parameter(Descriptor::kReceiver));
@@ -268,6 +553,22 @@ TF_BUILTIN(ObjectKeys, ObjectBuiltinsAssembler) {
268553
}
269554
}
270555

556+
TF_BUILTIN(ObjectValues, ObjectEntriesValuesBuiltinsAssembler) {
557+
TNode<JSObject> object =
558+
TNode<JSObject>::UncheckedCast(Parameter(Descriptor::kObject));
559+
TNode<Context> context =
560+
TNode<Context>::UncheckedCast(Parameter(Descriptor::kContext));
561+
GetOwnValuesOrEntries(context, object, CollectType::kValues);
562+
}
563+
564+
TF_BUILTIN(ObjectEntries, ObjectEntriesValuesBuiltinsAssembler) {
565+
TNode<JSObject> object =
566+
TNode<JSObject>::UncheckedCast(Parameter(Descriptor::kObject));
567+
TNode<Context> context =
568+
TNode<Context>::UncheckedCast(Parameter(Descriptor::kContext));
569+
GetOwnValuesOrEntries(context, object, CollectType::kEntries);
570+
}
571+
271572
// ES #sec-object.prototype.isprototypeof
272573
TF_BUILTIN(ObjectPrototypeIsPrototypeOf, ObjectBuiltinsAssembler) {
273574
Node* receiver = Parameter(Descriptor::kReceiver);

‎src/builtins/builtins-object.cc‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -395,31 +395,6 @@ BUILTIN(ObjectIsSealed) {
395395
return isolate->heap()->ToBoolean(result.FromJust());
396396
}
397397

398-
BUILTIN(ObjectValues) {
399-
HandleScope scope(isolate);
400-
Handle<Object> object = args.atOrUndefined(isolate, 1);
401-
Handle<JSReceiver> receiver;
402-
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
403-
Object::ToObject(isolate, object));
404-
Handle<FixedArray> values;
405-
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
406-
isolate, values, JSReceiver::GetOwnValues(receiver, ENUMERABLE_STRINGS));
407-
return *isolate->factory()->NewJSArrayWithElements(values);
408-
}
409-
410-
BUILTIN(ObjectEntries) {
411-
HandleScope scope(isolate);
412-
Handle<Object> object = args.atOrUndefined(isolate, 1);
413-
Handle<JSReceiver> receiver;
414-
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(isolate, receiver,
415-
Object::ToObject(isolate, object));
416-
Handle<FixedArray> entries;
417-
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
418-
isolate, entries,
419-
JSReceiver::GetOwnEntries(receiver, ENUMERABLE_STRINGS));
420-
return *isolate->factory()->NewJSArrayWithElements(entries);
421-
}
422-
423398
BUILTIN(ObjectGetOwnPropertyDescriptors) {
424399
HandleScope scope(isolate);
425400
Handle<Object> object = args.atOrUndefined(isolate, 1);

0 commit comments

Comments
 (0)