Closed
Bug 976527
Opened 10 years ago
Closed 9 years ago
Inspector slows down isbitcoindeadyet.com *a lot*
Categories
(DevTools :: Inspector, defect)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: paul, Unassigned)
References
Details
Attachments
(2 files)
STR: - open http://isbitcoindeadyet.com/ - page is smooth - open the toolbox with the webconsole - page is smooth - open the inspector - page is slow
Comment 1•10 years ago
|
||
The reason is because the site is applying transform properties constantly using JS to animate 20+ coins on the page, thus updating the node's properties, which in turn flashes and updates the markupview constantly. Making the whole process slow. IMO, the site should ideally use animations to do the same. Then the issue will not be there.
Comment 2•10 years ago
|
||
With local debugging: both the site and the devtools are slow. With remote debugging (via the Connect screen): the site is fine but the devtools are slow.
Reporter | ||
Comment 3•10 years ago
|
||
(In reply to Girish Sharma [:Optimizer] from comment #1) > IMO, the site should ideally use animations to do the same. Then the issue > will not be there. I don't think the devtools should only behave correctly on correctly coded websites :) We should not slow down the page no matter what.
Comment 4•10 years ago
|
||
With E10S coming down the pipe anyway, we'll have to make devtools run in a separate process, so at least, our devtools should never slow down sites anymore after we've migrated. Nevertheless, the inspector is really really slow on that page. We should probably throttle the updates made to the markup-view on markup mutations.
Comment 5•10 years ago
|
||
The only thing that can be done here to speed up things is to batch the packets for showing and flashing updates for the markup view (like I am doing for Storage Actor updates). But otherwise, this slowness is by design that we should show the changes and highlight the node live when any change in DOM happens.
Comment 6•10 years ago
|
||
Ok, so disabling completely the rule-view and computed-view doesn't seem to make any difference. However, disabling the markup-mutation observer in the markup-view makes both the site and the devtools responsive again. Several things can be tested from there on: - batch mutation packets - throttle the markup-view refresh rate - look at our MarkupContainer update logic, see if it can be made faster
Comment 7•10 years ago
|
||
After more investigation, it seems like what uses the most of the processing time is the templating part in the markup-view. Of course there are other things too (one candidate would also be the breadcrumb which refreshes at a very high rate too, for nothing), but the template part is the first part to be focused on.
Comment 8•10 years ago
|
||
https://dl.dropboxusercontent.com/u/714210/mozilla/bitcoin-site-inspector-profile.json This is a profile recorded with the browser toolbox's profiler. You can import it in your profiler. What this tells us is that in Templater.jsm: /** * Recursive function to walk the tree processing the attributes as it goes. * @param node the node to process. If you pass a string in instead of a DOM * element, it is assumed to be an id for use with document.getElementById() * @param data the data to use for node processing. */ Templater.prototype.processNode = function(node, data) {...} is the function that takes the most time to run when reacting to markup-mutations. In markup-view, ElementEditor.prototype.update is called when updating the node, and it does: // Get the attribute editor for each attribute that exists on // the node and show it. for (let attr of attrs) { let attribute = this._createAttribute(attr); if (!attribute.inplaceEditor) { attribute.style.removeProperty("display"); } } Which then calls: this.template("attribute", data);
Comment 9•10 years ago
|
||
"Profiling with platform data enabled" Shows that 15% of the time is spent on painting. This includes the flashing of 10+ rows. Screenshot : http://i.snag.gy/6Fs9n.jpg
Comment 10•10 years ago
|
||
On stepping through calls and stuff, I found that when an attribute change happens, in a markup mutation, we hide *all* the old attributes (i.e. the nodes representing the attributes) in the following lines : ElementEditor.prototype = { /** * Update the state of the editor from the node. */ update: function() { let attrs = this.node.attributes; if (!attrs) { return; } // Hide all the attribute editors, they'll be re-shown if they're // still applicable. Don't update attributes that are being // actively edited. let attrEditors = this.attrList.querySelectorAll(".attreditor"); for (let i = 0; i < attrEditors.length; i++) { if (!attrEditors[i].inplaceEditor) { attrEditors[i].style.display = "none"; } } Then we show them again (only which are applicable again) // Get the attribute editor for each attribute that exists on // the node and show it. for (let attr of attrs) { let attribute = this._createAttribute(attr); if (!attribute.inplaceEditor) { attribute.style.removeProperty("display"); } } I am assuming that this will cause a lot of reflows and recalculations and thus makes the whole process slow. It would be much faster to only remove and add removed and added attributes and just update the textContent of the changed attributes' values. PS: i removed the flash, stopped the breadcrumbs update on attribute change and disabled the sidebar. No visible perf improvement as suggested by Patrick.
Comment 11•10 years ago
|
||
(In reply to Girish Sharma [:Optimizer] from comment #10) Nope. I added "display: none" style on each of the row of markup view . Still the same slowness.
Comment 12•10 years ago
|
||
So, after more testing, the templating mechanism, is only half the problem on this site. As Optimizer said, we are hiding all attributes at each update, creating new ones (even if they have the same name and value) and showing them. Just by adding a simple test at the beginning of the function, I was able to improve the situation on this site quite dramatically. The templating still seems to be part of the problem as removing the call to template(...) and instead creating nodes with doc.creatElement and such seemed to make the page even faster. Let's keep in mind that this site is only one usecase, we should also look into pages that append and remove nodes in a rapid fashion. In this case, the _createAttributes function isn't going to be much of a problem. Last, I also changed a little bit the way the flashMutation function works as for now, it changes classNames everytime, even if not needed, and it seemed to have had some impacts on the performance. So, anyway, there are obvious performance improvements we can make on the markup-view, and I think there's enough in the comments of this bug to get started. I don't have time at the moment to work on it, so I'm leaving this unassigned.
Comment 13•9 years ago
|
||
This is another example of the inspector being slow on certain sites. This one is dhtmlconf.com Just opening the inspector makes the site really slow, and that's mostly coming from the animated particles that follow the mouse around on the site.
Comment 14•9 years ago
|
||
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #12) > So, after more testing, the templating mechanism, is only half the problem > on this site. > As Optimizer said, we are hiding all attributes at each update, creating new > ones (even if they have the same name and value) and showing them. > Just by adding a simple test at the beginning of the function, I was able to > improve the situation on this site quite dramatically. > The templating still seems to be part of the problem as removing the call to > template(...) and instead creating nodes with doc.creatElement and such > seemed to make the page even faster. > > Let's keep in mind that this site is only one usecase, we should also look > into pages that append and remove nodes in a rapid fashion. In this case, > the _createAttributes function isn't going to be much of a problem. > > Last, I also changed a little bit the way the flashMutation function works > as for now, it changes classNames everytime, even if not needed, and it > seemed to have had some impacts on the performance. > > So, anyway, there are obvious performance improvements we can make on the > markup-view, and I think there's enough in the comments of this bug to get > started. I don't have time at the moment to work on it, so I'm leaving this > unassigned. Do you have a WIP patch with these changes? It'd be worth landing some of the simple performance fixes, even if it doesn't fix all use cases.
Comment 15•9 years ago
|
||
Also FYI, the profiling data you took earlier could have been unreliable since this was before we had the capability to *not* load the debugger panel first. I would often see totally different results when profiling the same thing multiple times. Now if you open the browser toolbox, then switch the the profiler tab, then restart it will open on profiler by default next time and should hopefully be more reliable.
Comment 17•9 years ago
|
||
No sorry, these were just quick tests I had made, I didn't save any of it in a patch.
Flags: needinfo?(pbrosset)
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #4) > With E10S coming down the pipe anyway, we'll have to make devtools run in a > separate process, so at least, our devtools should never slow down sites > anymore after we've migrated. > > Nevertheless, the inspector is really really slow on that page. We should > probably throttle the updates made to the markup-view on markup mutations. Bug 1080884 was logged about this issue, which somehow becomes *worse* on E10S builds.
Comment 19•9 years ago
|
||
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #18) > (In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #4) > > With E10S coming down the pipe anyway, we'll have to make devtools run in a > > separate process, so at least, our devtools should never slow down sites > > anymore after we've migrated. > > > > Nevertheless, the inspector is really really slow on that page. We should > > probably throttle the updates made to the markup-view on markup mutations. > > Bug 1080884 was logged about this issue, which somehow becomes *worse* on > E10S builds. Unsurprising since the overhead of each RDP request is likely greater in e10s (I haven't actually measured, but it sure makes sense that inter-process communication is more expensive than intra-process communication).
Comment 20•9 years ago
|
||
The original example for this bug has an expired / parked domain name now.
Comment 21•9 years ago
|
||
Brian made a couple of important changes recently focusing specifically on the markup-view update performance. The site is closed now so it's not possible to test anymore, but we've done similar tests with js animations and there's now no apparent lag anymore (http://bgrins.github.io/devtools-demos/inspector/jquery-animation.html).
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•