Skip to content
forked from v8/v8

Commit

Permalink
Revert "[json] Postpone stack operation" from 11.8 branch
Browse files Browse the repository at this point in the history
along with two follow-up fixes that were backmerged previously.
This optimization needs some more time for getting stabilized.

This reverts the following commits:
79330c6
1783814
55bd79c

Bug: chromium:1485324, chromium:1481564, chromium:1481363
Change-Id: I3cdc741138c8194c3497ba78176faa4ace29d3f6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4886918
Commit-Queue: Michael Achenbach <[email protected]>
Auto-Submit: Jakob Kummerow <[email protected]>
Reviewed-by: Michael Achenbach <[email protected]>
Cr-Commit-Position: refs/branch-heads/11.8@{v8#14}
Cr-Branched-From: 935bdbf-refs/heads/11.8.172@{#1}
Cr-Branched-From: b82a911-refs/heads/main@{#89779}
  • Loading branch information
jakobkummerow authored and V8 LUCI CQ committed Sep 25, 2023
1 parent 6751500 commit acd92fd
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 83 deletions.
48 changes: 5 additions & 43 deletions src/json/json-stringifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class JsonStringifier {
Handle<Object> gap);

private:
enum Result { UNCHANGED, SUCCESS, EXCEPTION, NEED_STACK };
enum Result { UNCHANGED, SUCCESS, EXCEPTION };

bool InitializeReplacer(Handle<Object> replacer);
bool InitializeGap(Handle<Object> gap);
Expand Down Expand Up @@ -345,9 +345,7 @@ class JsonStringifier {
int indent_;
int part_length_;
int current_index_;
int stack_nesting_level_;
bool overflowed_;
bool need_stack_;

using KeyObject = std::pair<Handle<Object>, Handle<Object>>;
std::vector<KeyObject> stack_;
Expand Down Expand Up @@ -416,9 +414,7 @@ JsonStringifier::JsonStringifier(Isolate* isolate)
indent_(0),
part_length_(kInitialPartLength),
current_index_(0),
stack_nesting_level_(0),
overflowed_(false),
need_stack_(false),
stack_(),
key_cache_(isolate) {
one_byte_ptr_ = one_byte_array_;
Expand All @@ -437,11 +433,6 @@ MaybeHandle<Object> JsonStringifier::Stringify(Handle<Object> object,
return MaybeHandle<Object>();
}
Result result = SerializeObject(object);
if (result == NEED_STACK) {
indent_ = 0;
current_index_ = 0;
result = SerializeObject(object);
}
if (result == UNCHANGED) return factory()->undefined_value();
if (result == SUCCESS) {
if (overflowed_ || current_index_ > String::kMaxLength) {
Expand Down Expand Up @@ -603,14 +594,6 @@ Handle<JSReceiver> JsonStringifier::CurrentHolder(

JsonStringifier::Result JsonStringifier::StackPush(Handle<Object> object,
Handle<Object> key) {
if (!need_stack_) {
++stack_nesting_level_;
if V8_UNLIKELY (stack_nesting_level_ > 10) {
need_stack_ = true;
return NEED_STACK;
}
return SUCCESS;
}
StackLimitCheck check(isolate_);
if (check.HasOverflowed()) {
isolate_->StackOverflow();
Expand All @@ -637,13 +620,7 @@ JsonStringifier::Result JsonStringifier::StackPush(Handle<Object> object,
return SUCCESS;
}

void JsonStringifier::StackPop() {
if V8_LIKELY (!need_stack_) {
--stack_nesting_level_;
return;
}
stack_.pop_back();
}
void JsonStringifier::StackPop() { stack_.pop_back(); }

class CircularStructureMessageBuilder {
public:
Expand Down Expand Up @@ -759,9 +736,8 @@ Handle<String> JsonStringifier::ConstructCircularStructureErrorMessage(
bool MayHaveInterestingProperties(Isolate* isolate, JSReceiver object) {
for (PrototypeIterator iter(isolate, object, kStartAtReceiver);
!iter.IsAtEnd(); iter.Advance()) {
if (iter.GetCurrent()->map()->may_have_interesting_properties()) {
if (iter.GetCurrent()->map()->may_have_interesting_properties())
return true;
}
}
return false;
}
Expand All @@ -784,17 +760,11 @@ JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object,
if ((InstanceTypeChecker::IsJSReceiver(instance_type) &&
MayHaveInterestingProperties(isolate_, JSReceiver::cast(*object))) ||
InstanceTypeChecker::IsBigInt(instance_type)) {
if (!need_stack_ && stack_nesting_level_ > 0) {
need_stack_ = true;
return NEED_STACK;
}
need_stack_ = true;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate_, object, ApplyToJsonFunction(object, key), EXCEPTION);
}
}
if (!replacer_function_.is_null()) {
need_stack_ = true;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate_, object, ApplyReplacerFunction(object, key, initial_value),
EXCEPTION);
Expand Down Expand Up @@ -836,10 +806,6 @@ JsonStringifier::Result JsonStringifier::Serialize_(Handle<Object> object,
if (deferred_string_key) SerializeDeferredKey(comma, key);
return SerializeJSArray(Handle<JSArray>::cast(object), key);
case JS_PRIMITIVE_WRAPPER_TYPE:
if (!need_stack_) {
need_stack_ = true;
return NEED_STACK;
}
if (deferred_string_key) SerializeDeferredKey(comma, key);
return SerializeJSPrimitiveWrapper(
Handle<JSPrimitiveWrapper>::cast(object), key);
Expand Down Expand Up @@ -1144,17 +1110,13 @@ JsonStringifier::Result JsonStringifier::SerializeJSObject(
property = JSObject::FastPropertyAt(
isolate_, object, details.representation(), field_index);
} else {
if (!need_stack_) {
need_stack_ = true;
return NEED_STACK;
}
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate_, property,
Object::GetPropertyOrElement(isolate_, object, key_name), EXCEPTION);
}
Result result = SerializeProperty(property, comma, key_name);
if (!comma && result == SUCCESS) comma = true;
if (result == EXCEPTION || result == NEED_STACK) return result;
if (result == EXCEPTION) return result;
}
Unindent();
if (comma) NewLine();
Expand Down Expand Up @@ -1185,7 +1147,7 @@ JsonStringifier::Result JsonStringifier::SerializeJSReceiverSlow(
EXCEPTION);
Result result = SerializeProperty(property, comma, key);
if (!comma && result == SUCCESS) comma = true;
if (result == EXCEPTION || result == NEED_STACK) return result;
if (result == EXCEPTION) return result;
}
Unindent();
if (comma) NewLine();
Expand Down
20 changes: 0 additions & 20 deletions test/mjsunit/json.js
Original file line number Diff line number Diff line change
Expand Up @@ -531,23 +531,3 @@ assertEquals(`[
1,
2
]`, JSON.stringify([1,2], undefined, 1000000000000000));

let obj = { x: 0, y: 1 };
let other = {};
let called = false;
let count = 0;

obj.__defineGetter__(undefined, function() {
count++;
if (called) {
obj["__proto__"] = other["__lookupSetter__"];
} else {
obj["toLocaleString"] = other["__lookupSetter__"];
called = true;
}
});

assertEquals('{"x":0,"y":1}', JSON.stringify(obj));
assertEquals(1, count);
assertEquals('{"x":0,"y":1}', JSON.stringify(obj));
assertEquals(2, count);
20 changes: 0 additions & 20 deletions test/mjsunit/json2.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,23 +195,3 @@ var o = {};
o.somespecialproperty = 10;
o["\x19"] = 10;
assertThrows("JSON.parse('{\"somespecialproperty\":100, \"\x19\":10}')");

let exception_count = 0;
function foo(v) {
try {
v["set-i32"];
} catch (e) {
exception_count++;
}
try {
JSON.stringify(v);
} catch (e) {}
}
let obj1 = Object('2');
obj1.__proto__ = { toString: function () {} };
Object.defineProperty(obj1, "toString", {value: foo});
%EnsureFeedbackVectorForFunction(foo);
foo(obj1);
assertEquals(1, exception_count);
foo({obj1, b: { toJSON: function () {} }});
assertEquals(2, exception_count);

0 comments on commit acd92fd

Please sign in to comment.