Closed Bug 1434207 Opened 7 years ago Closed 3 years ago

Clearing innerHTML of the outputNode element can be slow

Categories

(DevTools :: Console, task, P3)

Product:
DevTools
Component:
Console
Type:
task

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: ochameau, Unassigned)

References

(Blocks 1 open bug)

Details

Bug 1419328 highlights that this particular line: https://searchfox.org/mozilla-central/source/devtools/client/webconsole/jsterm.js#1805 this.hud.outputNode.innerHTML = ""; can be very slow when the console has a lot of messages being already displayed. This code is called when we close the toolbox and can possibly delay its closing of make the browser UI jank. I'm wondering if that's any useful. The tool iframe is going to be removed anyway, with all its DOM content. And I imagine we unmount the react components somewhere else, right?
Here is a profile of DAMP running test from bug 1419328: https://perfht.ml/2Ft75T0 Clearing the innerHTML takes about 20% of the time spent during the "close" subtest.
Please don't use a HTML parser to clear a DOM element. The correct way is using element.textContent="" instead.
We might just switch to element.remove() instead
Priority: -- → P3
Product: Firefox → DevTools
Type: enhancement → task

Using element.remove() , .textContent = "" or simply removing the line don't have any positive impact.
Bug 1753177 might help here.
Doug could you share a profile closing a console that holds 10000 items, with your WIP patch for virtualization? I want to see if we can let this aside for now if this is going to be made better by virtualization.

Flags: needinfo?(dothayer)

Profile with virtualized list: https://share.firefox.dev/34ER0vq
Profile without: https://share.firefox.dev/34wGPJ5

We still take a little bit of time destroying it, but it's less than a frame, so I'm fine with saying bug 1753177 will close this.

Flags: needinfo?(dothayer)

That's great to hear. I'm making this bug depends on Bug 1753177 so we can assert that things are okay once we have virtualization.

Depends on: 1753177

Looking at DAMP results , closing the console with a lot of messages (custom.webconsole.close.DAMP), got 90% faster with Bug 1753177 🎉

Let's close this

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.