Closed
Bug 1016365
Opened 11 years ago
Closed 8 years ago
Add Debugger.Script.wasUsed property
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 1176880
People
(Reporter: nbp, Assigned: nbp)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
jimb
:
feedback+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
nbp
:
feedback+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
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
Comment 2•11 years ago
|
||
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)
Assignee | ||
Comment 3•10 years ago
|
||
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)
Comment 4•10 years ago
|
||
(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)
Assignee | ||
Comment 5•10 years ago
|
||
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 6•10 years ago
|
||
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+
Comment 7•10 years ago
|
||
We'll need test coverage for the multi-debugger case.
Flags: needinfo?(jimb)
Comment 8•10 years ago
|
||
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)
Comment 9•10 years ago
|
||
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)
Assignee | ||
Comment 10•10 years ago
|
||
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+
Comment 11•10 years ago
|
||
(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!)
Assignee | ||
Comment 12•10 years ago
|
||
(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.
Assignee | ||
Comment 13•8 years ago
|
||
(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.
Description
•