GC: make testGCOutOfMemory more reliable

RESOLVED FIXED in mozilla38

Status

()

defect
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: terrence, Assigned: sfink)

Tracking

Trunk
mozilla38
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Posted patch v0Splinter Review
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.
Attachment #720860 - Flags: review?(wmccloskey)
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.
Attachment #720860 - Flags: review?(wmccloskey)
Attachment #720911 - Flags: review?(terrence)
Assignee: terrence → sphink
The failure is in the second EVAL(), which means JS_EvaluateScript() is returning false.
Attachment #720922 - Flags: review?(terrence)
Attachment #720911 - Attachment is obsolete: true
Attachment #720911 - Flags: review?(terrence)
Depends on: 847686
Attachment #720922 - Flags: review?(terrence) → review+
No longer blocks: 706885
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.
Attachment #8549960 - Flags: review?(terrence)
Comment on attachment 8549960 [details] [diff] [review]
Re-enable testGCOutOfMemory

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

Most excellent!
Attachment #8549960 - Flags: review?(terrence) → review+
https://hg.mozilla.org/mozilla-central/rev/fc089bc9d516
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Duplicate of this bug: 847686
You need to log in before you can comment on or make changes to this bug.