Closed Bug 1016365 Opened 11 years ago Closed 8 years ago

Add Debugger.Script.wasUsed property

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1176880

People

(Reporter: nbp, Assigned: nbp)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

One of the runtime property which is interesting for Gaia, and potentially other web developers, would be to know how many time a function has been executed (approx. use-count). We can later add other info, such as if the function has been baseline/ion compiled or not. The goal of this bug is to provide raw data which is valuable for developers and can later be used by the dev-tools UI.
The use-count is not reliable enough to be used in an API, it is currently incremented in the interpreter, incremented in baseline (only if we are expecting to compile with Ion) and just never incremented in Ion. Worse, it is reset for a lot of reasons [1] (Bug 825268). What I suggest is to add: Debugger.startRecordingScriptUsage() Debugger.stopRecordingScriptUsage() Debugger.Script.wasUsed This way we would provide an API which is easy enough and similar the to CSS one, and which can be integrated in the dev-tools. (see Bug 949723) [1] http://dxr.mozilla.org/mozilla-central/search?q=%2Bfunction-ref%3AJSScript%3A%3AresetUseCount%28%29
Blocks: 949723
Summary: Add testing function to extract runtime properties of JSScripts. → Add Debugger.Script.wasUsed property
This sounds like it could nicely integrate with the tracelogging infrastructure. That already records the required information (plus lots more) and collects it in one place. @Hannes, do you think there's much work left to do until tracelogging can be enabled at runtime so it could be used by the Debugger?
Flags: needinfo?(hv1989)
This patch adds 2 properties: 1. Debugger.startRecordingScriptUsage This function reset the use count as well as the flag which record when a non-zero useCount is zeroed. It takes an extra argument which is a global for which all JSScript would be resetted. 2. Debugger.Script.wasUsed This property returns true / false if the function got used since the last time we started recording. I do not want to name the function "record" as it might later be miss-interepreted as a record & replay feature. This patch lacks documentation, as I am waiting on feedback to continue with this patch, but it should be enough for building dev-tools prototype out-of-it. I did not added the stop recording function as it is not necessary yet.
Attachment #8430158 - Flags: feedback?(jimb)
(In reply to Till Schneidereit [:till] from comment #2) > @Hannes, do you think there's much work left to do until tracelogging can be > enabled at runtime so it could be used by the Debugger? I think it still might take some time. Given the jit-tests and the performance issue that are currently still present. Looks like using the usecount for this seems like a possible interim short term solution.
Flags: needinfo?(hv1989)
Jimb: when have you plan to provide the feedback, to know if I should continue with this patch or just throw everything away?
Flags: needinfo?(jimb)
Comment on attachment 8430158 [details] [diff] [review] Debugger: Record function usage. Review of attachment 8430158 [details] [diff] [review]: ----------------------------------------------------------------- Nicolas and I discussed this on IRC. We agreed that the general idea of the feature is good, but I wanted to make sure it worked properly when there are multiple Debuggers observing a single compartment. We worked out the following algorithm: In JSRuntime: uint16_t scriptUsedGeneration; In JSScript: uint16_t usedGeneration; and change useCount to uint16_t, to avoid increasing the size of JSScript. In D.startRecordingScriptUsage: increment runtime's scriptUsedGeneration throw an error on overflow (fix in follow-up bug) note new value in D In S.wasUsed, where S is a Debugger.Script belonging to Debugger D return true if S->wasUsedGeneration is >= D's saved generation In emitUseCountIncrement, in debug mode: copy JSRuntime's scriptUsedGeneration to JSScript's usedGeneration
Attachment #8430158 - Flags: feedback?(jimb) → feedback+
We'll need test coverage for the multi-debugger case.
Flags: needinfo?(jimb)
Here are proposed docs; do they look reasonable? If so, please change the name of the accessor and method to match those given here.
Attachment #8435140 - Flags: feedback?(nicolas.b.pierron)
Grr, had to fix a few dumb things. No substantial change.
Attachment #8435140 - Attachment is obsolete: true
Attachment #8435140 - Flags: feedback?(nicolas.b.pierron)
Attachment #8435153 - Flags: feedback?(nicolas.b.pierron)
Comment on attachment 8435153 [details] [diff] [review] Proposed docs for Debugger.Script.prototype.wasUsed. Review of attachment 8435153 [details] [diff] [review]: ----------------------------------------------------------------- This is great, and this avoid the term "Record", which I guess we might want to keep for some record & replay feature.
Attachment #8435153 - Flags: feedback?(nicolas.b.pierron) → feedback+
(In reply to Jim Blandy :jimb from comment #6) > and change useCount to uint16_t, to avoid increasing the size of JSScript. Note that we increment the usecount in assembly (in baseline and in ionmonkey). Please update that relevant code to still work, correctly! (The one in ionmonkey is currently not tested. But can be enabled soonish!)
(In reply to Hannes Verschore [:h4writer] from comment #11) > (In reply to Jim Blandy :jimb from comment #6) > > and change useCount to uint16_t, to avoid increasing the size of JSScript. > > Note that we increment the usecount in assembly (in baseline and in > ionmonkey). Please update that relevant code to still work, correctly! (The > one in ionmonkey is currently not tested. But can be enabled soonish!) Thanks, we already considered it during our discussion ;) Which is why I suggested to separate it in two patches, one to refactor the usecount, and one to make use of the 16 extra bits. Also, as we stop incrementing the counter in IonMonkey, the counter is unlikely to overflow the 16 bits, and we have enough opportunities to reset it before reaching any overflow.
(In reply to Nicolas B. Pierron [:nbp] from comment #0) > One of the runtime property which is interesting for Gaia, and potentially > other web developers, would be to know how many time a function has been > executed (approx. use-count). This should be superseded by Bug 1176880 which implements real code coverage, and exposes it to the devtools.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: