Closed
Bug 1125259
Opened 10 years ago
Closed 10 years ago
Huge heap executing JavaScript (70x compared to V8)
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: rakudrama, Unassigned)
References
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file)
(deleted),
application/javascript
|
Details |
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.95 Safari/537.36
Steps to reproduce:
jsshell -f bug.js
Actual results:
The program prints some lines, mostly beginning "Mirrors read error: ..." and ends by throwing an exception. This is expected behaviour.
What is wrong is this program takes 10GB to run on jsshell or in FireFox.
On d8 (Chrome's V8 equivalent to jsshell) the heap is never bigger than 150MB.
jsshell --help reports: Version: JavaScript-C28.0
Expected results:
I would expect the two JavaScript engines execute in roughly the same amount of memory, say a factor of 2x-3x.
Comment 1•10 years ago
|
||
I see us get to about 1.2GB on Mac in a current opt shell. d8 gets to about 300MB.
Looking at what we're doing during the run, we're allocating a ton of stack strings from the new Error call in wrapException. If I just turn off stack capture for new Error, our memory usage never goes over about 120MB. Bug 1038238 would help.
OK, so that leaves an important question: is the script holding on to all those error objects, or are we just failing to GC expeditiously enough for some reason?
Comment 2•10 years ago
|
||
So I tried modifying wrapException by adding this line right after the |new Error| line:
var stack = wrapper.stack;
and then run in d8, memory usage goes to approximately 600MB before the program terminates.
Comment 3•10 years ago
|
||
And a bit of testing shows there are 106k calls to wrapException during the run.
None of this says what the lifetime of those Errors _should_ be.
Comment 4•10 years ago
|
||
OK, so in a debugger I do see a bunch of TOO_MUCH_MALLOC gcs as we go. So presumably the script is keeping the exceptions alive and they're not being collected, even though GC is triggering.
Comment 5•10 years ago
|
||
One other important difference that probably explains the lower V8 memory usage. V8's stacks stored on Error objects are only 10 frames deep as far as I can tell. We'll report the full stack as long as the stack string is less than 2^20 characters. So for deep enough stacks and exceptions kept alive, we'll presumably use more memory than V8 will.
Updated•10 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [MemShrink]
Comment 6•10 years ago
|
||
This code appears to be compiled from Dart. Does anyone know if this behavior is particular the the Dart program being compiled or inherent to all compiled Dart code? If it was the latter, that would certainly raise the priority.
Reporter | ||
Comment 7•10 years ago
|
||
Yes, this is compiled from Dart. I don't think the huge heap is a general problem.
This program is one of our tests that started failing on a test bot that was configured with 8GB RAM.
We do have some benchmarks programs that are much slower on jsshell than d8.
It would be great of we could get an understanding of that.
I will file those separately. Is there some way I should mark that bug report?
Comment 8•10 years ago
|
||
Stephen, just filing them and ccing me and Luke should be enough.
Comment 9•10 years ago
|
||
ooc, is the source Dart program explicitly creating and holding onto a lot of error objects or is the Error object an impl detail of some non-error Dart thing?
Comment 10•10 years ago
|
||
It looked to me like the Error objects here actually represented Dart exceptions....
Comment 11•10 years ago
|
||
Ah, then I guess the problem isn't Dart specific; the same issue would arise if a JS app was creating and saving a ton of Errors.
Comment 12•10 years ago
|
||
Yes, absolutely. Lots of Errors with deep stacks == badtimes on large string allocations.
Comment 13•10 years ago
|
||
> We'll report the full stack as long as the stack string is less than 2^20 characters.
Where is that 2^20 number in the code? That sounds like absurdly large limit. Could we just make it a lot smaller?
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 14•10 years ago
|
||
It's the super-poorly named "MaxReportedStackDepth" in js::ComputeStackString in jsexn.cpp.
We could certainly make it a lot smaller, but note that V8 is broken in the other extreme, imo: 10 stackframes is not enough to tell what the heck is going on with most sites...
Comment 15•10 years ago
|
||
> We could certainly make it a lot smaller, but note that V8 is broken in the
> other extreme, imo: 10 stackframes is not enough to tell what the heck is
> going on with most sites...
Sure. We could do something like limit it to 100 stack frames or 2^15 bytes, whichever is smaller?
Comment 16•10 years ago
|
||
Yeah, that seems fine. Console API caps to 200 stackframes, fwiw.
Comment 17•10 years ago
|
||
I just tested again now that bug 1038238 landed. Peak memory usage was ~167MB.
Do we want to resolve this bug now? Or are we still investigating why the Error objects are retained?
Comment 18•10 years ago
|
||
> I just tested again now that bug 1038238 landed. Peak memory usage was ~167MB.
Fixed! ;)
Afaict the Error objects are retained by the JS code itself...
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•