Closed Bug 1128528 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: JavaScript Engine, defect)

20 Branch
x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: alice0775, Assigned: jandem)

References

(Depends on 1 open bug)

Details

(Keywords: reproducible, testcase, Whiteboard: [MemShrink])

Attachments

(1 file)

+++ 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
Summary: JSON.parse crashes browser when decoding large JSON → JSON.parse not working properly when decoding large JSON
OS: Windows XP → Windows 7
This is Terrence's domain, I think.
Flags: needinfo?(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?
Flags: needinfo?(jdemooij)
Wow I can reproduce this and it's bad. We eat a ton of memory (> 10 GB). Will try to reduce this.
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.
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(terrence)
This is not JSON.parse but a custom parser written in JS, AFAICS.
Summary: JSON.parse not working properly when decoding large JSON → JSON parser written in JS not working properly when decoding large JSON
Attached patch PatchSplinter Review
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.
Flags: needinfo?(jdemooij)
Attachment #8558649 - Flags: review?(luke)
Comment on attachment 8558649 [details] [diff] [review]
Patch

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

Nice!
Attachment #8558649 - Flags: review?(luke) → review+
Whiteboard: [MemShrink]
https://hg.mozilla.org/mozilla-central/rev/a8ea4cc8215b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
QA Whiteboard: [good first verify]
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
You need to log in before you can comment on or make changes to this bug.