Closed
Bug 1001565
Opened 11 years ago
Closed 6 years ago
OOM crashes in Dromae's dom-modify cloneNode test
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bzbarsky, Assigned: jmaher)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
Joel ran into this in bug 987136 and bug 872788.
The short of it is that on Windows we're OOMing while running dom-modify.html. The stack is reliably coming via NodeBinding::cloneNode but at various depths into CloneAndAdopt.
I expect the problem is that we manage to execute lots of cloneNode calls during one second (good) and don't GC during the test (good, I guess?) so we run out of either RAM or VM space. Hard to tell which one from the crash dumps on try.
I'm not quite sure what the right course of action is here. If we're hitting RAM limits and have to GC (assuming we hooked that up) the test is not really all that useful as a performance test...
Reporter | ||
Comment 1•11 years ago
|
||
So one interesting option is to modify webrunner.js to trigger a GC after running each test. At least assuming we can inject something into there to give it that ability.
It's possible that we can get through a single copy of the cloneNode test without OOMing, just not 5 of them.
Assignee | ||
Comment 2•11 years ago
|
||
here is a change that appears to work:
http://hg.mozilla.org/users/jmaher_mozilla.com/tpain/rev/7916656e081c
Assignee | ||
Comment 3•11 years ago
|
||
:bz, is this what you were thinking? I see 10 green runs on try with it!
Flags: needinfo?(bzbarsky)
Reporter | ||
Comment 4•11 years ago
|
||
Yes, that's what I was thinking of in comment 1. Not sure we need all those dumps(), of course. ;)
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 5•11 years ago
|
||
this will probably affect all the numbers for dromaeo_dom, dromaeo_css. I am fine with the one time hit (if there is much of one). Open to all other changes you would suggest.
Reporter | ||
Comment 6•11 years ago
|
||
Comment on attachment 8413809 [details] [diff] [review]
official patch to gc between dromaeo subtests (1.0)
I'd expect a speedup, not a hit, since the main change would be to gc less during the tests themselves...
r=me
Attachment #8413809 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 7•11 years ago
|
||
landed on talos:
https://hg.mozilla.org/build/talos/rev/abff65130956
I will update the version of talos we use tonight or tomorrow (busy in meetings today)
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
Assignee | ||
Updated•6 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•