1016365 - Add Debugger.Script.wasUsed property

Status: RESOLVED DUPLICATE of bug 1176880

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

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.

Comment 1

[Nicolas B. Pierron [:nbp]]

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

Comment 2

[Till Schneidereit [:till]]

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?

Comment 3

[Nicolas B. Pierron [:nbp]]

Attachment: Debugger: Record function usage.

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.

Comment 4

[Hannes Verschore [:h4writer]]

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

Comment 5

[Nicolas B. Pierron [:nbp]]

Jimb: when have you plan to provide the feedback, to know if I should continue with this patch or just throw everything away?

Comment 6

[Jim Blandy :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

Comment 7

[Jim Blandy :jimb]

We'll need test coverage for the multi-debugger case.

Comment 8

[Jim Blandy :jimb]

Attachment: Proposed docs for Debugger.Script.prototype.wasUsed.

Here are proposed docs; do they look reasonable? If so, please change the name of the accessor and method to match those given here.

Comment 9

[Jim Blandy :jimb]

Attachment: Proposed docs for Debugger.Script.prototype.wasUsed.

Grr, had to fix a few dumb things. No substantial change.

Comment 10

[Nicolas B. Pierron [:nbp]]

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.

Comment 11

[Hannes Verschore [:h4writer]]

(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!)

Comment 12

[Nicolas B. Pierron [:nbp]]

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

Comment 13

[Nicolas B. Pierron [:nbp]]

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