772665 - Add console.profile API

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Benoit Girard (:BenWa)]

Webkit already has console.profile{End}(name). It instruments each JS function. I believe the API only works if you have the 'Profiles' pane of their tools open. It would be nice for web developers if we supported the same API. Here are my concerns:

- Can we just implement their API?
- Currently our profiler is design differently:
 (1) We can't nested profiling session, although we may be able to use markers to simulate having multiple profiling session. We could also just fix this limitation.
 (2) We have a circular buffer so we will drop samples after a while. We could have an increasing buffer size or we can just make it clear in the UI that you've dropped samples.
- To compensate for these limitations we have the benefit of being able to expend into the platform code.
- We don't the profiler built in so either we only enable it with the extension only or we make moving the profiler as a devtools a pre-req.

Comment 1

[Benoit Girard (:BenWa)]

http://fuelyourcoding.com/webkits-javascript-profiler-explained/

Comment 2

[Benoit Girard (:BenWa)]

Attachment: Example (View in webkit with 'Profiles' devtool)

Comment 3

[(no longer active)]

(In reply to comment #0)
> - Can we just implement their API?

Absolutely!  :-)

> - Currently our profiler is design differently:
>  (1) We can't nested profiling session, although we may be able to use markers
> to simulate having multiple profiling session. We could also just fix this
> limitation.

I like simple solutions more, and I like us to start using just a marker and see what goes wrong.  I don't suspect anything will, but see below.

>  (2) We have a circular buffer so we will drop samples after a while. We could
> have an increasing buffer size or we can just make it clear in the UI that
> you've dropped samples.

We should look into how V8 stores their profiles.  We could do dynamic page allocation to be able to store arbitrary profile sizes, but I don't necessarily think that's a requirement for this bug.  For now we can just crop profiles when we rotate once over our buffer.

> - To compensate for these limitations we have the benefit of being able to
> expend into the platform code.

Right, but I'm not sure if that's something that we want to expose to web developers...

> - We don't the profiler built in so either we only enable it with the extension
> only or we make moving the profiler as a devtools a pre-req.

The console.profile API may just be a no-op if the profiler is not active.  We don't need to advertize its usage to web developers until we have a profiler UI in place.

Comment 4

[(no longer active)]

Rob, who's our console.* guru?

Comment 5

[Benoit Girard (:BenWa)]

Can we find someone to own implementing console.profile? I already have quite a bit on my plate. 'tools/profiler/sampler.h' should expose the API that's needed to implement this so this should be easy.

Comment 6

[(no longer active)]

(In reply to comment #5)
> Can we find someone to own implementing console.profile? I already have quite a
> bit on my plate. 'tools/profiler/sampler.h' should expose the API that's needed
> to implement this so this should be easy.

See comment 4.  ;-)

Comment 7

[Benoit Girard (:BenWa)]

I just took a look and the code is here:
http://mxr.mozilla.org/mozilla-central/source/dom/base/ConsoleAPI.js

I think I can handle it:
- I'm going to expose the calls to console
- The calls will fire an observable event
- The profile add-on will pick up the observable event and will either ignore them if the user doesn't have the feature enable or will honor the events and insert markers.

Comment 8

[(no longer active)]

(In reply to comment #7)
> I just took a look and the code is here:
> http://mxr.mozilla.org/mozilla-central/source/dom/base/ConsoleAPI.js
> 
> I think I can handle it:
> - I'm going to expose the calls to console
> - The calls will fire an observable event
> - The profile add-on will pick up the observable event and will either ignore
> them if the user doesn't have the feature enable or will honor the events and
> insert markers.

Sounds reasonable to me.

Comment 9

[Benoit Girard (:BenWa)]

Attachment: Part 1: Expose console.profile{End}(name)

See Comment 7 for my design.

This patch should have no impact for any configuration without the profiler addon. It should send events into nothing.

Comment 10

[Benoit Girard (:BenWa)]

Attachment: Part 1: Expose console.profile{End}(name)

Comment 11

[Rob Campbell [:rc] (:robcee)]

(In reply to Ehsan Akhgari [:ehsan] from comment #4)
> Rob, who's our console.* guru?

that'd likely be mihai or, if pressed, myself. I'll ask mihai for a pass on this too.

Comment 12

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 640918 [details] [diff] [review]
Part 1: Expose console.profile{End}(name)

actually, we don't really need a mihai review on this. It's pretty simple.

Thanks BenWa!

Comment 13

[Benoit Girard (:BenWa)]

W00t. I was waiting for this r+ to finish the extension changes. Once the extension is ready I'll push this.

Comment 14

[Benoit Girard (:BenWa)]

Actually I changed my mind. Let's get this into a nightly ASAP. Right now 'Gecko Profiler' will simply echo the request and soon will handle it correctly.

Comment 15

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/83e30b5c8dba

Comment 16

[Benoit Girard (:BenWa)]

I have a prototype working on the extension.

Here's a profile of the sample page above. We should get a more interesting testcase:
http://people.mozilla.com/~bgirard/cleopatra/?report=bc2d6ed3b470fe61afca6c41f76c77c68c9b3948

Comment 17

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4410dced4a08

Comment 18

[Benoit Girard (:BenWa)]

Attachment: Add console.profile to test_consoleAPI.html

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/83e30b5c8dba
https://hg.mozilla.org/mozilla-central/rev/4410dced4a08