1128528 - JSON parser written in JS not working properly when decoding large JSON

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Alice0775 White]

+++ This bug was initially created as a clone of Bug #659333 +++

When I test Bug 659333, 

Steps to reproduce:
0. Copy to clipboard json text from attachment 8557891 [details]
1. Open http://json.parser.online.fr/
2. Paste with Ctrl+V into input field.

Actual Results:
Bad (High memory usage(1-2GB), nothning displays / "JS eval" displays / crashes)

Expected Results:
"String parse" and "JS eval" should display 

Regression window
Good (Peak memory less than 400MB and "String parse" and "JS eval" display as expected):
http://hg.mozilla.org/integration/mozilla-inbound/rev/88bb0e2f0373
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121009171503

Bad (High memory usage(1-2GB), nothning displays / "JS eval" displays / crashes)
http://hg.mozilla.org/integration/mozilla-inbound/rev/0761bc637081
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121009174802

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=88bb0e2f0373&tochange=0761bc637081

Triggered by: Bug 798624

Comment 1

[Jeff Walden [:Waldo]]

This is Terrence's domain, I think.

Comment 2

[Terrence Cole [:terrence]]

The primitive that that patch introduced -- StableCharPtr and out-of-lining -- is long, long gone. Jan, do you have any idea what in our current latin1/twobytes code paths might still be causing this behavior?

Comment 3

[Jan de Mooij [:jandem]]

Wow I can reproduce this and it's bad. We eat a ton of memory (> 10 GB). Will try to reduce this.

Comment 4

[Jan de Mooij [:jandem]]

The problem here is StrReplaceRegexpRemove calling ensureFlat, so we end up "undepending" a ton of dependent strings... With that fixed, the page is still not super fast but it's a lot better.

Comment 5

[Jan de Mooij [:jandem]]

This is not JSON.parse but a custom parser written in JS, AFAICS.

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Patch

Patch to use ensureLinear instead of ensureFlat in the following cases:

* StrReplaceRegexpRemove (the culprit here)
* Reflect.parse
* JSON.parse
* eval
* DebuggerGenericEval

The remaining ensureFlat calls are mostly error or ICU functions that expect a null-terminated string.

Comment 7

[Luke Wagner [:luke]]

Comment on attachment 8558649 [details] [diff] [review]
Patch

Review of attachment 8558649 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

Comment 8

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8ea4cc8215b

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a8ea4cc8215b

Comment 10

[iris]

I am late to comment on this one, but want to share a JSON Parser tool, which works well on New version of Firefox.
https://jsonformatter.org/json-parser