1001565 - OOM crashes in Dromae's dom-modify cloneNode test

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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...

Comment 1

[Boris Zbarsky [:bzbarsky]]

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.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

here is a change that appears to work:
http://hg.mozilla.org/users/jmaher_mozilla.com/tpain/rev/7916656e081c

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

:bz, is this what you were thinking?  I see 10 green runs on try with it!

Comment 4

[Boris Zbarsky [:bzbarsky]]

Yes, that's what I was thinking of in comment 1.  Not sure we need all those dumps(), of course.  ;)

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: official patch to gc between dromaeo subtests (1.0)

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

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

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

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)