Closed Bug 705020 Opened 8 years ago Closed 7 years ago

remove enable-js-diagnostics flag from nightly mozconfig

Categories

(Firefox for Android :: General, defect)

Product:
Firefox for Android
Component:
General
Platform:
x86
macOS
Type:
defect
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 21
Tracking Flags:
Tracking Status
firefox20 --- fixed
firefox21 --- fixed

People

(Reporter: dougt, Assigned: kats)

Details

Attachments

(2 files)

patch v.1
851 bytes, patch
mbrubeck
: review+
Details | Diff | Splinter Review
Remove from mobile/xul
886 bytes, patch
dougt
: review+
bajaj
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
From email:

me:
we are building with --enable-js-diagnostics in our nightly builds.  anyone know what this means w.r.t perf?  Also, if this is a high cost, do we know the value?

David Mandelin:
I don't think it's too expensive. But I also doubt it would really be useful to you--that stuff was for trying to diagnose GC crashes, and mostly it just told us that all of our actionable ideas about what was causing crashes weren't right.
Attached patch patch v.1 — — Splinter Review
Assignee: nobody → doug.turner
Attachment #576657 - Flags: review?(mbrubeck)
Attachment #576657 - Flags: review?(mbrubeck) → review+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
This option still exists in the mobile/xul nightly mozconfig, and should probably be removed there. The reason I care is because it is now in the mozilla-aurora tree as well, http://hg.mozilla.org/releases/mozilla-aurora/file/489f8fb0b4fa/mobile/xul/config/mozconfigs/android/nightly even though it's explicitly supposed to only be on nightlies. Clearly something went wrong.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 699203 [details] [diff] [review]
Remove from mobile/xul

[Approval Request Comment]
Bug caused by (feature/regressing bug #): version bump when aurora went from 19 -> 20
User impact if declined: in theory, aurora builds from XUL fennec will have some js diagnostic stuff turned on, but i don't think anybody is building that so it doesn't really matter. this does show up when i diff the FF19 tree at the end of aurora and the start of beta so that's kind of annoying
Testing completed (on m-c, etc.): none
Risk to taking this patch (and alternatives if risky): should be no risk, this isn't even being used anywhere really
String or UUID changes made by this patch: none
Attachment #699203 - Flags: approval-mozilla-aurora?
Actually, sorry, it's unrelated to the 19 -> 20 version bump, I misread the hg history. It's been on aurora for a long time.
Comment on attachment 699203 [details] [diff] [review]
Remove from mobile/xul

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

i do not care about mobile/xul, but the patch lgtm.  i'd love to r+ a patch that removed mobile/xul from m-c.
Attachment #699203 - Flags: review?(doug.turner) → review+
This was NPOTB - for future patches like this, please can you use DONTBUILD :-)
(d326a33ad6f6 that is)
Assignee: doug.turner → bugmail.mozilla
https://hg.mozilla.org/mozilla-central/rev/d326a33ad6f6
Status: REOPENED → RESOLVED
Closed: 8 years ago → 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Attachment #699203 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.