686571 - Add preference for turning on JSOPTION_PCCOUNTS

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

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.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: 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.)

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Add preferences for turning on pccounts in the browser

Comment 3

[Brian Hackett [Laid off!]]

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.

Comment 4

[David Mandelin [:dmandelin]]

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.

Comment 5

[Steve Fink [:sfink] [:s:]]

(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.

Comment 6

[Steve Fink [:sfink] [:s:]]

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

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/8ad3742f0f83
https://hg.mozilla.org/mozilla-central/rev/4236dfa03448

Comment 8

[Cedric Vivier [:cedricv]]

Attachment: Bug 686571 - Expose new javascript.options.pccounts.(content|chrome) prefs defaults. r=dmandelin

Comment 9

[Ed Morley [:emorley]]

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 :-)

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/09b9a8009763