Closed Bug 899853 Opened 12 years ago Closed 3 years ago

The JS profiler needs to aggregate and squash the JS frames belonging to other tabs

Categories

(Core :: Gecko Profiler, defect, P2)

Product:
Core
Component:
Gecko Profiler
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: ehsan.akhgari, Unassigned)

References

Details

Not sure if we have an existing bug on this, but currently the JS profiler shows frames from all of the tabs running in your browser, which is quite sub-optimal since most web developers will only be interested in the tab that they're trying to profile. We should only show them frames belonging to the current JSContext.
We have a WONTFIX bug somewhere. The problem is, since we don't have per-tab isolation, such behavior will produce inaccurate results especially when some other tab (or an addon) causes jank all over the place.
It might be useful as a non-default option though.
(In reply to comment #1) > We have a WONTFIX bug somewhere. The problem is, since we don't have per-tab > isolation, such behavior will produce inaccurate results especially when some > other tab (or an addon) causes jank all over the place. What do you mean by per-tab isolation? I think it depends on how you define the expected result. Why would we try to optimize our developer tools for finding a problem in an add-on? We drop things like native frames already. My impression is that the goal here is to build a useful tool for profiling JS for *web* developers. Am I missing something?
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #3) > What do you mean by per-tab isolation? I mean that slow code in tab A can cause jank in tab B. Or is my understanding incorrect?
(In reply to comment #4) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #3) > > What do you mean by per-tab isolation? > > I mean that slow code in tab A can cause jank in tab B. Or is my understanding > incorrect? Yes, and I'm trying to argue that it is irrelevant. The Web platform tries to provide you with an isolated sandbox in which your web application runs, and it's possible to reason about the performance characteristics of your application's code without caring about what other tabs in your browser, or other programs running side by side your browser, are doing. Note that if our goal was to build a tool to analyze why Firefox was slow in doing something, we would build something which gave you an exact idea of what Firefox was doing, be it running your application's code, some other tab's code, some add-on, some internal background task, etc. But the goal of the JS profiler is to give you insight into why *your* application is slow, and help you fix those problem. Telling the web developers that hey this other tab caused a jank right now does nothing to help them figure out how to prevent janks caused by their own code, which is the most they can ever hope to be able to fix. I hope this makes my position clearer. If it doesn't, please feel free to let me know. I'm also curious to know your thoughts on this.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #5) > (In reply to comment #4) > > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #3) > > > What do you mean by per-tab isolation? > > > > I mean that slow code in tab A can cause jank in tab B. Or is my understanding > > incorrect? > > Yes, and I'm trying to argue that it is irrelevant. To be honest, the more I'm trying to come up with my arguments the more I tend to agree with you on this issue. My original argument was about the case when developer is debugging reporter performance issues but in that case they will have to ask their users to profile the app and send them data (only to find out that some other tab is causing the jank). But that's not going to happen in the real world, I'm afraid. In addition to that, since we can detect when jank is caused by some other tab, we could notify developers about that without showing symbols from all tabs. I'm curious what Rob thinks about that but I think it could be a good thing to do.
Flags: needinfo?(rcampbell)
s/reporter/reported
Priority: -- → P2
(In reply to Anton Kovalyov (:anton) from comment #6) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #5) > > (In reply to comment #4) > > > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #3) > > > > What do you mean by per-tab isolation? > > > > > > I mean that slow code in tab A can cause jank in tab B. Or is my understanding > > > incorrect? > > > > Yes, and I'm trying to argue that it is irrelevant. > > To be honest, the more I'm trying to come up with my arguments the more I > tend to agree with you on this issue. My original argument was about the > case when developer is debugging reporter performance issues but in that > case they will have to ask their users to profile the app and send them data > (only to find out that some other tab is causing the jank). But that's not > going to happen in the real world, I'm afraid. > > In addition to that, since we can detect when jank is caused by some other > tab, we could notify developers about that without showing symbols from all > tabs. > > I'm curious what Rob thinks about that but I think it could be a good thing > to do. My initial idea for a JS Profiler for Firefox imagined getting a report of everything that was running in the targeted tab. Because of the way the Gecko Profiler worked and because of some discussions we had around the early implementation of it, we decided to show everything (since a slow-down might not originate in the tab you're running, it was reasoned that you might want to know about that). I think there's good value in showing only the target tab's JS. It might even be the default display. Showing everything feels like something we might want to do in a Browser Profiler but that might not be what a web developer is interested in. I think we should do this and see if we like it.
Flags: needinfo?(rcampbell)
(In reply to comment #8) > (In reply to Anton Kovalyov (:anton) from comment #6) > > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #5) > > > (In reply to comment #4) > > > > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #3) > > > > > What do you mean by per-tab isolation? > > > > > > > > I mean that slow code in tab A can cause jank in tab B. Or is my understanding > > > > incorrect? > > > > > > Yes, and I'm trying to argue that it is irrelevant. > > > > To be honest, the more I'm trying to come up with my arguments the more I > > tend to agree with you on this issue. My original argument was about the > > case when developer is debugging reporter performance issues but in that > > case they will have to ask their users to profile the app and send them data > > (only to find out that some other tab is causing the jank). But that's not > > going to happen in the real world, I'm afraid. Agreed! > > In addition to that, since we can detect when jank is caused by some other > > tab, we could notify developers about that without showing symbols from all > > tabs. Note that is not strictly true, it's mostly true if we're running content JS though... But that's an important point to keep in mind. It's just not possible to present everything we do to handle one tab to the developers because a lot of that is not JS, and exposing things like internal Gecko function names is not going to help much. > > I'm curious what Rob thinks about that but I think it could be a good thing > > to do. > > My initial idea for a JS Profiler for Firefox imagined getting a report of > everything that was running in the targeted tab. Because of the way the Gecko > Profiler worked and because of some discussions we had around the early > implementation of it, we decided to show everything (since a slow-down might > not originate in the tab you're running, it was reasoned that you might want to > know about that). > > I think there's good value in showing only the target tab's JS. It might even > be the default display. Showing everything feels like something we might want > to do in a Browser Profiler but that might not be what a web developer is > interested in. Note that would be very difficult in a multi-process world, and it would actually not be very useful, since content running in a different process won't block your tab in most cases. I'd be cautious of exposing features which we would have to rip out completely with e10s. I feel like it would probably be more interesting for a Browser Profiler to look at chrome JS, since that's probably what add-on authors are interested in. > I think we should do this and see if we like it. Thanks a lot for the discussion guys! :-)
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #9) > Note that is not strictly true, it's mostly true if we're running content JS > though... But that's an important point to keep in mind. It's just not > possible to present everything we do to handle one tab to the developers > because a lot of that is not JS, and exposing things like internal Gecko > function names is not going to help much. We already have an option to expose non-JS symbols. It was added for our Metro UX team since they can't use BenWa's profiler addon because add-ons are not supported there yet.
(In reply to comment #10) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #9) > > > Note that is not strictly true, it's mostly true if we're running content JS > > though... But that's an important point to keep in mind. It's just not > > possible to present everything we do to handle one tab to the developers > > because a lot of that is not JS, and exposing things like internal Gecko > > function names is not going to help much. > > We already have an option to expose non-JS symbols. It was added for our Metro > UX team since they can't use BenWa's profiler addon because add-ons are not > supported there yet. Sure, but that is not necessarily something that will benefit web/add-on developers. I seriously question that anything exposing internal Gecko symbol names will ever be beneficial for those people.
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #11) > Sure, but that is not necessarily something that will benefit web/add-on > developers. I can't tell for addon developers, but for web dev, this is not really beneficial. > I seriously question that anything exposing internal Gecko > symbol names will ever be beneficial for those people. At first, I, as a web dev, don't want to have to learn about Firefox internals to analyze and improve the performance of my website/web app. All of that knowledge has 2 issues: 1) it is Firefox-specific (and people will prefer to learn and use perf improvements that work across different browsers) 2) the knowledge will change as Firefox internals change and too few developers have the time to keep up with internal Gecko changes. It might be that for some additional perf improvements, Gecko-specific knowledge is valuable, but I believe that in the majority of cases, people have lots of work to do before getting to that point. And for this work to do before, Gecko symbols are unnecessary noise. Overall, I agree with what Ehsan said on this bug. As a web developer, I want a profiler that only tells me about what's happening in my website/web app in the tab I'm running in. And I only want symbol that relate to the code I have written or the libraries I imported.
(In reply to David Bruant from comment #12) > Overall, I agree with what Ehsan said on this bug. As a web developer, I > want a profiler that only tells me about what's happening in my website/web > app in the tab I'm running in. And I only want symbol that relate to the > code I have written or the libraries I imported. I agree that it's the right thing to do for the profiler we present in the devtools. But again sometimes seeing the Gecko internal will tell you that the reason your canvas painting is slower then you think is because you're hitting the scaling code when your expecting to be blitting images. Although I agree most users wont be able to understand that by looking at a large profile.
Note that there is information that we can expose to make it easier for web developers to optimize their code other than the JS frames. For example I filed a bug about exposing how long DOM calls take a while ago, but I can't find it right now.
(In reply to comment #13) > (In reply to David Bruant from comment #12) > > Overall, I agree with what Ehsan said on this bug. As a web developer, I > > want a profiler that only tells me about what's happening in my website/web > > app in the tab I'm running in. And I only want symbol that relate to the > > code I have written or the libraries I imported. > > I agree that it's the right thing to do for the profiler we present in the > devtools. But again sometimes seeing the Gecko internal will tell you that the > reason your canvas painting is slower then you think is because you're hitting > the scaling code when your expecting to be blitting images. Although I agree > most users wont be able to understand that by looking at a large profile. No Benoit, that is information that is meaningful to us Gecko hackers. Our function names won't make sense to anybody else.
(In reply to Benoit Girard (:BenWa) from comment #13) > But again sometimes seeing the Gecko internal will tell you that > the reason your canvas painting is slower then you think is because you're > hitting the scaling code when your expecting to be blitting images. I don't understand what that means. If I'm told in a way or another "your canvas painting is slower than you think because you're hitting the scaling code": Should I care if I don't notice a slowdown? Should I change my page layout? Should I refactor my JS? Avoid a given built-in? (lots of other questions) Don't bother answering. My point is that more information is useful only if you know what to make of it. In that case, it wouldn't. I think a good guiding principle on whether information should be provided is: can an experienced webdev without knowledge about specific browser internals make actionable use of that information? And the number of times a userland JS time function function is called and how long did it take is an actionable information. What's happening in other tabs and addons cannot really be actionable (beyond closing the tabs and disabling the noisy addons) I imagine that for browser internal information, if you don't combine the information with actionable suggestions (like "stop calling native function X that often" or "call this function X only after function Y"), most of the time, the answer is no. Does that sound like a good guiding principle?
I think we're all violently agreeing with each other at this point.
Hey Rob, any idea when we're going to fix this bug? I'm trying to get bug 785440 fixed, which would give us an awesome profiler for web developers, but people keep telling me that work won't be much useful unless we fix this bug, which makes sense to me.
Flags: needinfo?(rcampbell)
is there anyway we can get this from SPS Profiler? Alternatively, we'll have to parse URIs for every frame in a tab and that's going to be a bit... icky. If that's what we have to do though, we can do that.
Flags: needinfo?(rcampbell)
OS: Mac OS X → All
Hardware: x86 → All
(In reply to Rob Campbell [:rc] (:robcee) from comment #19) > is there anyway we can get this from SPS Profiler? Not that I know of. > Alternatively, we'll have > to parse URIs for every frame in a tab and that's going to be a bit... icky. > If that's what we have to do though, we can do that. The URI won't be good enough since you may have two tabs open with the same URI (such as gmail!). I think what we want is to base this stuff on the JSRuntime* or something. You should check with somebody from the JS team on what the right unique "ID" to hang this off of would be.
we discussed this a couple of days ago and came up with a couple of ideas for this: 1) annotate pseudo-stacks with originating window id, if we can get it. (probably easier and consistent with some of our filtering techniques in the Console) 2) profile only for given JS contexts. (might require deeper integration with SpiderMonkey) I'd be happy with either solution if we can get one to work.
Aren't the pseduostacks captured inside the JS engine? It seems to me that accessing the window ID from there might be a pain (but I could be wrong.)
Core.
Component: Developer Tools: Profiler → Gecko Profiler
Product: Firefox → Core
Note that we should show these frames as <other tab> or something in the tree, similar to how bug 1108843 does <graphics> etc instead of filtering them away out right. Filtering out samples has the effect of skewing the profiled data and percentages we calculate from it, which is no bueno.
Summary: The JS profiler needs to filter the JS frames belonging to the current tab → The JS profiler needs to aggregate and squash the JS frames belonging to other tabs

Thanks to Fission and the Profiler's "Web Developer" settings, we can now show only the threads (including JS) of a chosen web page.
So I'll call this bug here fixed.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.