Closed Bug 987666 Opened 6 years ago Closed 5 years ago

Remove the dynamic rooting analysis

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: terrence, Assigned: terrence)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

Attached patch rm_root_analysis-v0.diff (obsolete) — Splinter Review
I think I got all of it?
Attachment #8396360 - Flags: review?(sphink)
Comment on attachment 8396360 [details] [diff] [review]
rm_root_analysis-v0.diff

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

Wow. I had no idea that had crept into so much stuff!
Attachment #8396360 - Flags: review?(sphink) → review-
Comment on attachment 8396360 [details] [diff] [review]
rm_root_analysis-v0.diff

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

Wow. I had no idea that had crept into so much stuff!

Ok. I really need to go to sleep. I meant r+, not r- !
Attachment #8396360 - Flags: review- → review+
Rebased to tip. Final diffstat:

  40 files changed, 22 insertions(+), 653 deletions(-)
Attachment #8396360 - Attachment is obsolete: true
Attachment #8398510 - Flags: review+
Keywords: checkin-needed
OMG, SkipRoot leaked into the browser!?!
Attachment #8398570 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/fbca45e65930
https://hg.mozilla.org/mozilla-central/rev/a926b8938e9a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I missed the IsPoisoned assertions. 150 lines worth of them. Spread all over the place. Hopefully that's the last for real this time.

https://tbpl.mozilla.org/?tree=Try&rev=e838e744545e
Attachment #8399463 - Flags: review?(sphink)
https://tbpl.mozilla.org/?tree=Try&rev=e41b2d977d63

Gah! That leaked into the browser too.
Comment on attachment 8399463 [details] [diff] [review]
missed_some_root_analysis_removal-v0.diff

We discussed this at the GC meeting today and decided to convert these sites over to JS_CRASH_DIAGNOSTICS.
Attachment #8399463 - Flags: review?(sphink) → review-
Terrence, what's the status here? Does any of this need uplifting for 31 (incl. wrt maintainability for ESR)?
Blocks: GC.stability
No longer blocks: 885550
I think we only bits from the prior patch we wanted to keep was removal from configure.in. Here is a patch to do that removal.
Attachment #8495436 - Flags: review?(sphink)
Attachment #8495436 - Flags: review?(sphink) → review+
https://hg.mozilla.org/mozilla-central/rev/51563b7c1a8f
Status: REOPENED → RESOLVED
Closed: 6 years ago5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.