Browser-based gczeal is a trap for the unwary

RESOLVED FIXED

Status

()

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Waldo, Assigned: Waldo)

Tracking

(Depends on: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
Created attachment 502730 [details] [diff] [review]
Reset gczeal(0) on page unload to get back to a speedy state

It changes GC zeal for all time, not until the next test occurs.  This is bad.  It's not at all what I would have expected, and it caused a change I made to produce inexplicable tinderbox timeouts, at least until I debugged this.  Nobody should have to make this mistake.
Attachment #502730 - Flags: review?(bclary)
Future-luke thanks you.

Updated

8 years ago
Attachment #502730 - Flags: review?(bclary) → review+
(Assignee)

Comment 2

8 years ago
http://hg.mozilla.org/tracemonkey/rev/e272d471f4c4

Existing tests will continue to reset gczeal, don't think that needs to be fixed now (or indeed ever, actually, but we might still want to do it so people learning by example don't think entirely wrongly).
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/e272d471f4c4
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Comment 4

8 years ago
fyi, with this change it is no longer possible to run the full suite under gczeal 2.
(Assignee)

Comment 5

8 years ago
Hm, point; that's not ideal.  We could sample gczeal at test start, then restore to that original value at test end.  And perhaps we should change the current imperative mode of gczealing to something functional that could restore the original zeal, not just clear to 0 -- withZeal(2, callback) -- although I'm less certain about whether this or something else would be preferable.  Mind filing the followup to do this?

Updated

8 years ago
Depends on: 628310
You need to log in before you can comment on or make changes to this bug.