Closed Bug 924706 Opened 12 years ago Closed 12 years ago

Make sure JSGC_BEGIN callback runs again if we reset an incremental GC

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch reset-fix (obsolete) — Splinter Review
Andrew wants to use the JSGC_BEGIN callback to purge some data stored weakly in a cache. We do this in a few places in the JS engine. The problem is that the following can happen: begin incremental GC, clearing the cache store X something in the cache reset the GC sweep X try to access X via the cache I think it makes the most sense to re-run the JSGC_BEGIN callback when we reset. Here's a patch to do that, although it's untested. I've pushed it to try here: https://tbpl.mozilla.org/?tree=Try&rev=32ca202303fb Andrew, would you mind testing this to see if it makes your problem go away? I'll ask jonco to review if everything seems to work.
It has been running bc for 26 minutes without a crash, and without those other changes we made, so it looks like it works, thanks!
Blocks: 905382
The try results don't look too good. I'll take a look today or tomorrow. I might have messed up the scheduling of the next GC.
Attached patch reset-fix v2Splinter Review
Try looks good.
Attachment #814666 - Attachment is obsolete: true
Attachment #821330 - Flags: review?(jcoppeard)
Hooray, thanks Bill!
Comment on attachment 821330 [details] [diff] [review] reset-fix v2 Review of attachment 821330 [details] [diff] [review]: ----------------------------------------------------------------- Great, not calling the begin callback when we restart the GC sounds like a really bad idea.
Attachment #821330 - Flags: review?(jcoppeard) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: