Commit ae18522e authored by Leszek Swirski's avatar Leszek Swirski Committed by V8 LUCI CQ

Revert "[runtime] Optimise paired instance type checks"

This reverts commit 92edf9a1.

Reason for revert: Breaks mjsunit/es6/proxies-json on GCStress https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/39619/overview

Original change's description:
> [runtime] Optimise paired instance type checks
>
> Clang doesn't optimise over handle derefs. Change the ValueSerializer
> and the JsonStringifier to use InstanceType directly for checks.
> This CL squeezes another 1.5% of JSON.stringify in local benchmarks.
>
> Drive-by-fix:
> - Avoid a few more derefs in the JsonStringifier
> - Make JsonStringifier::SerializeJSArray a bit more readable
>
> Change-Id: I37626a6d92a8d9275611a4e6d1d908f2e0c6d43b
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3247637
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#77697}

Change-Id: I127dd5832b9caceb0d5b74631eede274551405e0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3260511
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77700}
parent 0446ab7c
...@@ -102,7 +102,6 @@ class JsonStringifier { ...@@ -102,7 +102,6 @@ class JsonStringifier {
V8_INLINE static bool DoNotEscape(Char c); V8_INLINE static bool DoNotEscape(Char c);
V8_INLINE void NewLine(); V8_INLINE void NewLine();
V8_NOINLINE void NewLineOutline();
V8_INLINE void Indent() { indent_++; } V8_INLINE void Indent() { indent_++; }
V8_INLINE void Unindent() { indent_--; } V8_INLINE void Unindent() { indent_--; }
V8_INLINE void Separator(bool first); V8_INLINE void Separator(bool first);
...@@ -384,10 +383,8 @@ JsonStringifier::Result JsonStringifier::StackPush(Handle<Object> object, ...@@ -384,10 +383,8 @@ JsonStringifier::Result JsonStringifier::StackPush(Handle<Object> object,
{ {
DisallowGarbageCollection no_gc; DisallowGarbageCollection no_gc;
Object raw_obj = *object; for (size_t i = 0; i < stack_.size(); ++i) {
size_t size = stack_.size(); if (*stack_[i].second == *object) {
for (size_t i = 0; i < size; ++i) {
if (*stack_[i].second == raw_obj) {
AllowGarbageCollection allow_to_return_error; AllowGarbageCollection allow_to_return_error;
Handle<String> circle_description = Handle<String> circle_description =
ConstructCircularStructureErrorMessage(key, i); ConstructCircularStructureErrorMessage(key, i);
...@@ -525,14 +522,9 @@ JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object, ...@@ -525,14 +522,9 @@ JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object,
isolate_->stack_guard()->HandleInterrupts().IsException(isolate_)) { isolate_->stack_guard()->HandleInterrupts().IsException(isolate_)) {
return EXCEPTION; return EXCEPTION;
} }
if (!object->IsSmi()) { if (object->IsJSReceiver() || object->IsBigInt()) {
InstanceType instance_type = ASSIGN_RETURN_ON_EXCEPTION_VALUE(
HeapObject::cast(*object).map().instance_type(); isolate_, object, ApplyToJsonFunction(object, key), EXCEPTION);
if (InstanceTypeChecker::IsJSReceiver(instance_type) ||
InstanceTypeChecker::IsBigInt(instance_type)) {
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate_, object, ApplyToJsonFunction(object, key), EXCEPTION);
}
} }
if (!replacer_function_.is_null()) { if (!replacer_function_.is_null()) {
ASSIGN_RETURN_ON_EXCEPTION_VALUE( ASSIGN_RETURN_ON_EXCEPTION_VALUE(
...@@ -545,8 +537,7 @@ JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object, ...@@ -545,8 +537,7 @@ JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object,
return SerializeSmi(Smi::cast(*object)); return SerializeSmi(Smi::cast(*object));
} }
InstanceType instance_type = HeapObject::cast(*object).map().instance_type(); switch (HeapObject::cast(*object).map().instance_type()) {
switch (instance_type) {
case HEAP_NUMBER_TYPE: case HEAP_NUMBER_TYPE:
if (deferred_string_key) SerializeDeferredKey(comma, key); if (deferred_string_key) SerializeDeferredKey(comma, key);
return SerializeHeapNumber(Handle<HeapNumber>::cast(object)); return SerializeHeapNumber(Handle<HeapNumber>::cast(object));
...@@ -581,16 +572,16 @@ JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object, ...@@ -581,16 +572,16 @@ JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object,
case SYMBOL_TYPE: case SYMBOL_TYPE:
return UNCHANGED; return UNCHANGED;
default: default:
if (InstanceTypeChecker::IsString(instance_type)) { if (object->IsString()) {
if (deferred_string_key) SerializeDeferredKey(comma, key); if (deferred_string_key) SerializeDeferredKey(comma, key);
SerializeString(Handle<String>::cast(object)); SerializeString(Handle<String>::cast(object));
return SUCCESS; return SUCCESS;
} else { } else {
DCHECK(object->IsJSReceiver()); DCHECK(object->IsJSReceiver());
if (HeapObject::cast(*object).IsCallable()) return UNCHANGED; if (object->IsCallable()) return UNCHANGED;
// Go to slow path for global proxy and objects requiring access checks. // Go to slow path for global proxy and objects requiring access checks.
if (deferred_string_key) SerializeDeferredKey(comma, key); if (deferred_string_key) SerializeDeferredKey(comma, key);
if (InstanceTypeChecker::IsJSProxy(instance_type)) { if (object->IsJSProxy()) {
return SerializeJSProxy(Handle<JSProxy>::cast(object), key); return SerializeJSProxy(Handle<JSProxy>::cast(object), key);
} }
return SerializeJSObject(Handle<JSObject>::cast(object), key); return SerializeJSObject(Handle<JSObject>::cast(object), key);
...@@ -658,55 +649,45 @@ JsonStringifier::Result JsonStringifier::SerializeJSArray( ...@@ -658,55 +649,45 @@ JsonStringifier::Result JsonStringifier::SerializeJSArray(
builder_.AppendCharacter('['); builder_.AppendCharacter('[');
Indent(); Indent();
uint32_t i = 0; uint32_t i = 0;
if (replacer_function_.is_null() && length > 0) { if (replacer_function_.is_null()) {
StackLimitCheck interrupt_check(isolate_);
const uint32_t kInterruptLength = 4000;
uint32_t limit = std::min(length, kInterruptLength);
const uint32_t kMaxAllowedFastPackedLength =
std::numeric_limits<uint32_t>::max() - kInterruptLength;
STATIC_ASSERT(FixedArray::kMaxLength < kMaxAllowedFastPackedLength);
switch (object->GetElementsKind()) { switch (object->GetElementsKind()) {
case PACKED_SMI_ELEMENTS: { case PACKED_SMI_ELEMENTS: {
Handle<FixedArray> elements(FixedArray::cast(object->elements()), Handle<FixedArray> elements(FixedArray::cast(object->elements()),
isolate_); isolate_);
while (true) { StackLimitCheck interrupt_check(isolate_);
for (; i < limit; i++) { while (i < length) {
Separator(i == 0);
SerializeSmi(Smi::cast(elements->get(i)));
}
if (i >= length) break;
DCHECK_LT(limit, kMaxAllowedFastPackedLength);
limit = std::min(length, limit + kInterruptLength);
if (interrupt_check.InterruptRequested() && if (interrupt_check.InterruptRequested() &&
isolate_->stack_guard()->HandleInterrupts().IsException( isolate_->stack_guard()->HandleInterrupts().IsException(
isolate_)) { isolate_)) {
return EXCEPTION; return EXCEPTION;
} }
Separator(i == 0);
SerializeSmi(Smi::cast(elements->get(i)));
i++;
} }
break; break;
} }
case PACKED_DOUBLE_ELEMENTS: { case PACKED_DOUBLE_ELEMENTS: {
// Empty array is FixedArray but not FixedDoubleArray.
if (length == 0) break;
Handle<FixedDoubleArray> elements( Handle<FixedDoubleArray> elements(
FixedDoubleArray::cast(object->elements()), isolate_); FixedDoubleArray::cast(object->elements()), isolate_);
while (true) { StackLimitCheck interrupt_check(isolate_);
for (; i < limit; i++) { while (i < length) {
Separator(i == 0);
SerializeDouble(elements->get_scalar(i));
}
if (i >= length) break;
DCHECK_LT(limit, kMaxAllowedFastPackedLength);
limit = std::min(length, limit + kInterruptLength);
if (interrupt_check.InterruptRequested() && if (interrupt_check.InterruptRequested() &&
isolate_->stack_guard()->HandleInterrupts().IsException( isolate_->stack_guard()->HandleInterrupts().IsException(
isolate_)) { isolate_)) {
return EXCEPTION; return EXCEPTION;
} }
Separator(i == 0);
SerializeDouble(elements->get_scalar(i));
i++;
} }
break; break;
} }
case PACKED_ELEMENTS: { case PACKED_ELEMENTS: {
Handle<Object> old_length(object->length(), isolate_); Handle<Object> old_length(object->length(), isolate_);
for (i = 0; i < length; i++) { while (i < length) {
if (object->length() != *old_length || if (object->length() != *old_length ||
object->GetElementsKind() != PACKED_ELEMENTS) { object->GetElementsKind() != PACKED_ELEMENTS) {
// Fall back to slow path. // Fall back to slow path.
...@@ -715,15 +696,20 @@ JsonStringifier::Result JsonStringifier::SerializeJSArray( ...@@ -715,15 +696,20 @@ JsonStringifier::Result JsonStringifier::SerializeJSArray(
Separator(i == 0); Separator(i == 0);
Result result = SerializeElement( Result result = SerializeElement(
isolate_, isolate_,
handle(FixedArray::cast(object->elements()).get(i), isolate_), i); Handle<Object>(FixedArray::cast(object->elements()).get(i),
isolate_),
i);
if (result == UNCHANGED) { if (result == UNCHANGED) {
builder_.AppendCString("null"); builder_.AppendCString("null");
} else if (result != SUCCESS) { } else if (result != SUCCESS) {
return result; return result;
} }
i++;
} }
break; break;
} }
// The FAST_HOLEY_* cases could be handled in a faster way. They resemble
// the non-holey cases except that a lookup is necessary for holes.
default: default:
break; break;
} }
...@@ -767,43 +753,39 @@ JsonStringifier::Result JsonStringifier::SerializeArrayLikeSlow( ...@@ -767,43 +753,39 @@ JsonStringifier::Result JsonStringifier::SerializeArrayLikeSlow(
return SUCCESS; return SUCCESS;
} }
namespace {
V8_INLINE bool CanFastSerializeJSObject(JSObject raw_object, Isolate* isolate) {
DisallowGarbageCollection no_gc;
if (raw_object.map().IsCustomElementsReceiverMap()) return false;
if (!raw_object.HasFastProperties()) return false;
auto roots = ReadOnlyRoots(isolate);
return raw_object.elements() == roots.empty_fixed_array() ||
raw_object.elements() == roots.empty_slow_element_dictionary();
}
} // namespace
JsonStringifier::Result JsonStringifier::SerializeJSObject( JsonStringifier::Result JsonStringifier::SerializeJSObject(
Handle<JSObject> object, Handle<Object> key) { Handle<JSObject> object, Handle<Object> key) {
HandleScope handle_scope(isolate_); HandleScope handle_scope(isolate_);
Result stack_push = StackPush(object, key); Result stack_push = StackPush(object, key);
if (stack_push != SUCCESS) return stack_push; if (stack_push != SUCCESS) return stack_push;
if (property_list_.is_null() && CanFastSerializeJSObject(*object, isolate_)) {
if (property_list_.is_null() &&
!object->map().IsCustomElementsReceiverMap() &&
object->HasFastProperties() &&
(object->elements() == ReadOnlyRoots(isolate_).empty_fixed_array() ||
object->elements() ==
ReadOnlyRoots(isolate_).empty_slow_element_dictionary())) {
DCHECK(!object->IsJSGlobalProxy()); DCHECK(!object->IsJSGlobalProxy());
DCHECK(!object->HasIndexedInterceptor()); DCHECK(!object->HasIndexedInterceptor());
DCHECK(!object->HasNamedInterceptor()); DCHECK(!object->HasNamedInterceptor());
Map map = object->map(); Handle<Map> map(object->map(), isolate_);
builder_.AppendCharacter('{'); builder_.AppendCharacter('{');
Indent(); Indent();
bool comma = false; bool comma = false;
for (InternalIndex i : map.IterateOwnDescriptors()) { for (InternalIndex i : map->IterateOwnDescriptors()) {
Handle<Name> name(map.instance_descriptors(isolate_).GetKey(i), isolate_); Handle<Name> name(map->instance_descriptors(isolate_).GetKey(i),
isolate_);
// TODO(rossberg): Should this throw? // TODO(rossberg): Should this throw?
if (!name->IsString()) continue; if (!name->IsString()) continue;
Handle<String> key_name = Handle<String>::cast(name); Handle<String> key_name = Handle<String>::cast(name);
PropertyDetails details = PropertyDetails details =
map.instance_descriptors(isolate_).GetDetails(i); map->instance_descriptors(isolate_).GetDetails(i);
if (details.IsDontEnum()) continue; if (details.IsDontEnum()) continue;
Handle<Object> property; Handle<Object> property;
if (details.location() == PropertyLocation::kField && if (details.location() == PropertyLocation::kField &&
map == object->map()) { *map == object->map()) {
DCHECK_EQ(kData, details.kind()); DCHECK_EQ(kData, details.kind());
FieldIndex field_index = FieldIndex::ForDescriptor(map, i); FieldIndex field_index = FieldIndex::ForDescriptor(*map, i);
property = JSObject::FastPropertyAt(object, details.representation(), property = JSObject::FastPropertyAt(object, details.representation(),
field_index); field_index);
} else { } else {
...@@ -1029,10 +1011,6 @@ bool JsonStringifier::DoNotEscape(uint16_t c) { ...@@ -1029,10 +1011,6 @@ bool JsonStringifier::DoNotEscape(uint16_t c) {
void JsonStringifier::NewLine() { void JsonStringifier::NewLine() {
if (gap_ == nullptr) return; if (gap_ == nullptr) return;
NewLineOutline();
}
void JsonStringifier::NewLineOutline() {
builder_.AppendCharacter('\n'); builder_.AppendCharacter('\n');
for (int i = 0; i < indent_; i++) builder_.AppendCString(gap_); for (int i = 0; i < indent_; i++) builder_.AppendCString(gap_);
} }
......
...@@ -988,12 +988,12 @@ class V8_EXPORT_PRIVATE FlatStringReader : public Relocatable { ...@@ -988,12 +988,12 @@ class V8_EXPORT_PRIVATE FlatStringReader : public Relocatable {
inline base::uc32 Get(int index) const; inline base::uc32 Get(int index) const;
template <typename Char> template <typename Char>
inline Char Get(int index) const; inline Char Get(int index) const;
int length() const { return length_; } int length() { return length_; }
private: private:
Handle<String> str_; Handle<String> str_;
bool is_one_byte_; bool is_one_byte_;
int const length_; int length_;
const void* start_; const void* start_;
}; };
......
...@@ -413,8 +413,7 @@ Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) { ...@@ -413,8 +413,7 @@ Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) {
} }
DCHECK(object->IsHeapObject()); DCHECK(object->IsHeapObject());
InstanceType instance_type = HeapObject::cast(*object).map().instance_type(); switch (HeapObject::cast(*object).map().instance_type()) {
switch (instance_type) {
case ODDBALL_TYPE: case ODDBALL_TYPE:
WriteOddball(Oddball::cast(*object)); WriteOddball(Oddball::cast(*object));
return ThrowIfOutOfMemory(); return ThrowIfOutOfMemory();
...@@ -434,7 +433,7 @@ Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) { ...@@ -434,7 +433,7 @@ Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) {
Handle<JSArrayBufferView> view = Handle<JSArrayBufferView>::cast(object); Handle<JSArrayBufferView> view = Handle<JSArrayBufferView>::cast(object);
if (!id_map_.Find(view) && !treat_array_buffer_views_as_host_objects_) { if (!id_map_.Find(view) && !treat_array_buffer_views_as_host_objects_) {
Handle<JSArrayBuffer> buffer( Handle<JSArrayBuffer> buffer(
InstanceTypeChecker::IsJSTypedArray(instance_type) view->IsJSTypedArray()
? Handle<JSTypedArray>::cast(view)->GetBuffer() ? Handle<JSTypedArray>::cast(view)->GetBuffer()
: handle(JSArrayBuffer::cast(view->buffer()), isolate_)); : handle(JSArrayBuffer::cast(view->buffer()), isolate_));
if (!WriteJSReceiver(buffer).FromMaybe(false)) return Nothing<bool>(); if (!WriteJSReceiver(buffer).FromMaybe(false)) return Nothing<bool>();
...@@ -442,10 +441,10 @@ Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) { ...@@ -442,10 +441,10 @@ Maybe<bool> ValueSerializer::WriteObject(Handle<Object> object) {
return WriteJSReceiver(view); return WriteJSReceiver(view);
} }
default: default:
if (InstanceTypeChecker::IsString(instance_type)) { if (object->IsString()) {
WriteString(Handle<String>::cast(object)); WriteString(Handle<String>::cast(object));
return ThrowIfOutOfMemory(); return ThrowIfOutOfMemory();
} else if (InstanceTypeChecker::IsJSReceiver(instance_type)) { } else if (object->IsJSReceiver()) {
return WriteJSReceiver(Handle<JSReceiver>::cast(object)); return WriteJSReceiver(Handle<JSReceiver>::cast(object));
} else { } else {
ThrowDataCloneError(MessageTemplate::kDataCloneError, object); ThrowDataCloneError(MessageTemplate::kDataCloneError, object);
...@@ -688,20 +687,20 @@ Maybe<bool> ValueSerializer::WriteJSArray(Handle<JSArray> array) { ...@@ -688,20 +687,20 @@ Maybe<bool> ValueSerializer::WriteJSArray(Handle<JSArray> array) {
// structure of the elements changing. // structure of the elements changing.
switch (array->GetElementsKind()) { switch (array->GetElementsKind()) {
case PACKED_SMI_ELEMENTS: { case PACKED_SMI_ELEMENTS: {
DisallowGarbageCollection no_gc; Handle<FixedArray> elements(FixedArray::cast(array->elements()),
FixedArray elements = FixedArray::cast(array->elements()); isolate_);
for (i = 0; i < length; i++) WriteSmi(Smi::cast(elements.get(i))); for (; i < length; i++) WriteSmi(Smi::cast(elements->get(i)));
break; break;
} }
case PACKED_DOUBLE_ELEMENTS: { case PACKED_DOUBLE_ELEMENTS: {
// Elements are empty_fixed_array, not a FixedDoubleArray, if the array // Elements are empty_fixed_array, not a FixedDoubleArray, if the array
// is empty. No elements to encode in this case anyhow. // is empty. No elements to encode in this case anyhow.
if (length == 0) break; if (length == 0) break;
DisallowGarbageCollection no_gc; Handle<FixedDoubleArray> elements(
FixedDoubleArray elements = FixedDoubleArray::cast(array->elements()); FixedDoubleArray::cast(array->elements()), isolate_);
for (i = 0; i < length; i++) { for (; i < length; i++) {
WriteTag(SerializationTag::kDouble); WriteTag(SerializationTag::kDouble);
WriteDouble(elements.get_scalar(i)); WriteDouble(elements->get_scalar(i));
} }
break; break;
} }
......
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