Closed Bug 812393 Opened 8 years ago Closed 8 years ago

Sweep strings and scripts incrementally

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox19 --- wontfix
firefox20 --- fixed

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(2 files)

The original incremental sweeping patch in bug 729760 was artificially restricted to shapes and types. I'm now thinking that the reasons for this may have been wrong. We should sweep strings and scripts this way too.

(The rationale for not sweeping objects incrementally is still sound. In that case, XPConnect's finalizer functions NULL out a pointer from a C++ object to its JS reflector. If we did that incrementally, XPConnect could "resurrect" a JS object that we already had decided to sweep using that pointer.)

The only strings we sweep on the main thread are external strings. They call a special function in their finalizer that frees some memory or decrements a reference count. Either way, I think this is safe to call incrementally.

Scripts are more complicated. The finalizer for them removes the script from a number of tables and possibly destroys some JIT code. However, it's impossible to obtain a script that's about to be finalized from these tables: we never iterate over the tables, and the script is always used as a key.

I did a tryserver run and it's looking good so far:
https://tbpl.mozilla.org/?tree=Try&rev=d2d9a9f9af99
Attachment #682290 - Flags: review?(jcoppeard)
The patch for scripts.
Attachment #682291 - Flags: review?(jcoppeard)
Comment on attachment 682290 [details] [diff] [review]
sweep strings incrementally

Looks good, I'm pretty sure the original implementation finalized strings before shapes though... has something else changed to make this work?
Attachment #682290 - Flags: review?(jcoppeard) → review+
Comment on attachment 682291 [details] [diff] [review]
sweep scripts incrementally

Looks good.
Attachment #682291 - Flags: review?(jcoppeard) → review+
https://hg.mozilla.org/mozilla-central/rev/166eefa41f21
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Depends on: 815542
You need to log in before you can comment on or make changes to this bug.