The default bug view has changed. See this FAQ.

Add console.profile API

RESOLVED FIXED in mozilla16

Status

()

Core
Gecko Profiler
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

unspecified
mozilla16
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
http://fuelyourcoding.com/webkits-javascript-profiler-explained/
(Assignee)

Comment 2

5 years ago
Created attachment 640819 [details]
Example (View in webkit with 'Profiles' devtool)
(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.
Rob, who's our console.* guru?
(Assignee)

Comment 5

5 years ago
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.
(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.  ;-)
(Assignee)

Comment 7

5 years ago
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.
(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.
(Assignee)

Comment 9

5 years ago
Created attachment 640917 [details] [diff] [review]
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.
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Attachment #640917 - Flags: review?(rcampbell)
(Assignee)

Comment 10

5 years ago
Created attachment 640918 [details] [diff] [review]
Part 1: Expose console.profile{End}(name)
Attachment #640917 - Attachment is obsolete: true
Attachment #640917 - Flags: review?(rcampbell)
Attachment #640918 - Flags: review?(rcampbell)
(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 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!
Attachment #640918 - Flags: review?(rcampbell) → review+
(Assignee)

Comment 13

5 years ago
W00t. I was waiting for this r+ to finish the extension changes. Once the extension is ready I'll push this.
(Assignee)

Comment 14

5 years ago
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.
(Assignee)

Comment 15

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/83e30b5c8dba
Target Milestone: --- → mozilla16
(Assignee)

Comment 16

5 years ago
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
Target Milestone: mozilla16 → ---
(Assignee)

Updated

5 years ago
Depends on: 773428
(Assignee)

Comment 17

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4410dced4a08
(Assignee)

Comment 18

5 years ago
Created attachment 641603 [details] [diff] [review]
Add console.profile to test_consoleAPI.html
https://hg.mozilla.org/mozilla-central/rev/83e30b5c8dba
https://hg.mozilla.org/mozilla-central/rev/4410dced4a08
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.