The default bug view has changed. See this FAQ.

Add preference for turning on JSOPTION_PCCOUNTS

RESOLVED FIXED in mozilla9

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: sfink, Assigned: sfink)

Tracking

unspecified
mozilla9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

6 years ago
I would like a preference to turn on the JSOPTION_PCCOUNTS option (which records execution counts for every Javascript opcode run), so that the collected information can be surfaced through addons.

This is probably the wrong component. This isn't a bug in the preferences system, it's a specific preference.
(Assignee)

Comment 1

6 years ago
Created attachment 560048 [details] [diff] [review]
Prevent crash when running code with pccounts on if compiled with pccounts off

Need to check whether a pcLengths collection area exists before writing to it (for when pccounts are toggled after compilation.)
Attachment #560048 - Flags: review?(bhackett1024)
(Assignee)

Comment 2

6 years ago
Created attachment 560049 [details] [diff] [review]
Add preferences for turning on pccounts in the browser
(Assignee)

Updated

6 years ago
Attachment #560049 - Flags: review?(dmandelin)
Comment on attachment 560048 [details] [diff] [review]
Prevent crash when running code with pccounts on if compiled with pccounts off

If you want to make the JITScript::pcLengths independent from cx->hasRunOption(JSOPTION_PCCOUNT), can you remove the JSOPTION_PCCOUNT test entirely?  It seems like it's pointless now.

Eventually you could make it so that changing the PCCOUNT option's value immediately discards all jitcode in the compartment/runtime, so that future jitcode will reflect the current setting.
Attachment #560048 - Flags: review?(bhackett1024) → review+
Comment on attachment 560049 [details] [diff] [review]
Add preferences for turning on pccounts in the browser

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

That options code is starting to get repetitive. It'd be great if someone went in and added some abstraction there.
Attachment #560049 - Flags: review?(dmandelin) → review+
Component: Preferences → General
OS: Linux → All
Product: Firefox → Core
QA Contact: preferences → general
Hardware: x86_64 → All
(Assignee)

Comment 5

6 years ago
(In reply to Brian Hackett from comment #3)
> Comment on attachment 560048 [details] [diff] [review]
> Prevent crash when running code with pccounts on if compiled with pccounts
> off
> 
> If you want to make the JITScript::pcLengths independent from
> cx->hasRunOption(JSOPTION_PCCOUNT), can you remove the JSOPTION_PCCOUNT test
> entirely?  It seems like it's pointless now.

Well, not entirely. If you compile with JSOPTION_PCCOUNT on, both the interpreter and method JIT will accumulate until you turn it off. With your change, it would continue accumulating and later queries would see counts that include time after you turned the option off.

On the other hand, turning it on won't do anything until a recompile, so you can't usefully bracket periods of time you care about with option on then off anyway. I think your way is better because it's a little simpler.

> Eventually you could make it so that changing the PCCOUNT option's value
> immediately discards all jitcode in the compartment/runtime, so that future
> jitcode will reflect the current setting.

Yes, just like the debugger.
OS: All → Linux
Hardware: All → x86_64
(Assignee)

Updated

6 years ago
Whiteboard: [inbound]
(Assignee)

Comment 6

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/32004f4bcf6f
https://hg.mozilla.org/integration/mozilla-inbound/rev/4236dfa03448
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ad3742f0f83
https://hg.mozilla.org/mozilla-central/rev/8ad3742f0f83
https://hg.mozilla.org/mozilla-central/rev/4236dfa03448
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
Created attachment 560653 [details] [diff] [review]
Bug 686571 - Expose new javascript.options.pccounts.(content|chrome) prefs defaults. r=dmandelin
Attachment #560653 - Flags: review?(dmandelin)
Attachment #560653 - Flags: review?(dmandelin) → review+
Keywords: checkin-needed

Updated

6 years ago
Assignee: nobody → sphink
Component: General → JavaScript Engine
QA Contact: general → general
Keywords: checkin-needed
OS: Linux → All
Hardware: x86_64 → All
Comment on attachment 560653 [details] [diff] [review]
Bug 686571 - Expose new javascript.options.pccounts.(content|chrome) prefs defaults. r=dmandelin

Followup sent to inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/09b9a8009763

Cedric, to save time for future patches (this one is fine for now), could you set your hgrc to include the author automatically, along the lines of:
http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

Thanks :-)
https://hg.mozilla.org/mozilla-central/rev/09b9a8009763
You need to log in before you can comment on or make changes to this bug.