847579 - GC: make testGCOutOfMemory more reliable

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Attachment: v0

This test counts how many objects it can allocate, then GC's, then asserts that it can safely allocate max/4 objects. If we increase the max heap size to 2MiB from 512KiB, then the test normally fails. If we insert a few milliseconds of sleep after doing the GC then the test succeeds again.

The problem is that the background sweeping may still be running when we start trying to allocate again. This is currently masked by the fact that the heap limit it sets is very close to the starting limit such that max/4 is extremely small and likely to fit in the foreground finalized things.

This patch simply triggers a second GC after the first to force us to wait for background finalization to finish. We should probably make the mechanism more robust, but the tree is closed and this is needed now, now, now.

Comment 1

[Terrence Cole [:terrence]]

Comment on attachment 720860 [details] [diff] [review]
v0

Bill pointed out in IRC that refillFreeList will block on the background finalizer for us, so this isn't actually the problem. I also don't see this anymore working off of tip instead of the GGC. The allocation paths should be the same between the two builds after we hit the threshold the first time, so I need to figure out what is going wrong there first. Will re-request after I figure out what is happening.

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: disable testGCOutOfMemory temporarily

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: disable testGCOutOfMemory temporarily

The failure is in the second EVAL(), which means JS_EvaluateScript() is returning false.

Comment 4

[Steve Fink [:sfink] [:s:]]

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

(r=terrence via IRC)

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: Re-enable testGCOutOfMemory

This bug was [leave-open] because it was a "temporary" test disable. I tracked down what was happening and left a big comment describing what was going on.

Note that I don't fully understand the test case. Eg, why does it need to explicitly wipe out 'array' with "array = []; array.push(0);"? It should be dead after the OOM exception anyway. Then again, why does it not seem to free memory with the JS_GC() call?

Oh well.

Comment 7

[Terrence Cole [:terrence]]

Comment on attachment 8549960 [details] [diff] [review]
Re-enable testGCOutOfMemory

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

Most excellent!

Comment 8

[Steve Fink [:sfink] [:s:]]

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

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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