Have a separate preference to only enable the gc/cc notifications

VERIFIED FIXED in mozilla16

Status

()

Core
DOM
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Optimizer, Assigned: terrence)

Tracking

({dev-doc-needed})

Trunk
mozilla16
dev-doc-needed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
there should be a separate preference, like js.mem.notify which will only send a notification on each GC/CC and not dump into the error console.
(Assignee)

Comment 1

5 years ago
Created attachment 642038 [details] [diff] [review]
v0

I think this is roughly what is needed.  Bill, does this make sense?
Assignee: general → terrence
Status: NEW → ASSIGNED
Attachment #642038 - Flags: review?(wmccloskey)
Component: JavaScript Engine → DOM
Comment on attachment 642038 [details] [diff] [review]
v0

This looks fine, but I think we should wait to land it until MemChaser has been updated to set the new preference. Can you talk to Henrik about this?
Attachment #642038 - Flags: review?(wmccloskey) → review+
(Reporter)

Comment 3

5 years ago
Actually, I would also be releasing an addon, Graphical Timeline of Events, that visually displays the GC/CC. So if this could land in 16, it would be very helpful.
This request has been filed originally as bug 752861. Given that an already reviewed patch is attached here, I will dupe it to this one.
Duplicate of this bug: 752861
I talked to Henrik about this. He's going to update MemChaser to set the new preference. A few days after that (so people have time to update), we'll land this patch on trunk and Aurora.
https://hg.mozilla.org/integration/mozilla-inbound/rev/173b3e481297

Optimizer suggested the idea of generating the JSON if mem.log or mem.notify are set. Then MemChaser and other addons can migrate to mem.notify. At that point, we can make mem.log only generate error console output and not JSON. That sounded good, so I made those changes and landed so that this makes it into 16.
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/173b3e481297
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Used this pref in memchaser for a while now and everything looks fine for Nightly, Aurora, and Beta. Have we updated MDN for this change?
Status: RESOLVED → VERIFIED
Keywords: dev-doc-needed
(Reporter)

Comment 10

5 years ago
I can also confirm, it works. There was a plan to completely stop sending notification along with the previous pref, should I file a new bug for that ?, enough time has passed..
(Assignee)

Comment 11

5 years ago
(In reply to Girish Sharma [:Optimizer] from comment #10)
> I can also confirm, it works. There was a plan to completely stop sending
> notification along with the previous pref, should I file a new bug for that
> ?, enough time has passed..

Please do!  We might as well get it into this release if you and Henrik are both ready.
(Reporter)

Updated

5 years ago
Blocks: 797066
(In reply to Girish Sharma [:Optimizer] from comment #10)
> I can also confirm, it works. There was a plan to completely stop sending
> notification along with the previous pref, should I file a new bug for that
> ?, enough time has passed..

Is this really a good idea? I mean without this pref it would not be possible anymore for normal users to get the stats logged to the Error console. There are plenty of situations when such type of usage is necessary. Without it there would be no easy way anymore and you would be forced to install an extension which connects to the API. Personally I would not recommend to remove the preference.
(Reporter)

Comment 13

5 years ago
(In reply to Henrik Skupin (:whimboo) from comment #12)
> Is this really a good idea? I mean without this pref it would not be
> possible anymore for normal users to get the stats logged to the Error
> console. There are plenty of situations when such type of usage is
> necessary. Without it there would be no easy way anymore and you would be
> forced to install an extension which connects to the API. Personally I would
> not recommend to remove the preference.

there is javascript.options.mem.log for logging. Its just that it should *only* log and not send notification also, as for notification there is javascript.options.mem.notify

so to an end user, the difference will be zero.
Ok, re-read comment 6 and you are right. So yes, please file a bug to get the sending of JSON data removed when mem.log has been set. Sorry for mixing it up.
(Reporter)

Comment 15

5 years ago
Did that. Fixed that. Already landed.
See bug 797066
;)
You need to log in before you can comment on or make changes to this bug.