Closed Bug 1753177 Opened 4 months ago Closed 2 months ago

Use a virtualized list for the webconsole

Categories

(DevTools :: Console, task)

task

Tracking

(firefox101 fixed)

RESOLVED FIXED
101 Branch
Tracking Status
firefox101 --- fixed

People

(Reporter: nchevobbe, Assigned: dthayer)

References

(Depends on 2 open bugs, Blocks 1 open bug, Regressed 1 open bug)

Details

(Whiteboard: dt-console-perf-2022)

Attachments

(4 files, 1 obsolete file)

This is probably the most efficient way we could make the console faster.

Let's see how we could do that, and what the challenges are

Impact

Possible Issues

  • Variable message element height
  • Layout changes caused by user action:
    • Expanding/Collapsing objects
    • Expanding/Collapsing stacks
    • Expanding/Collapsing groups
    • Filtering
    • Showing sidebar
    • Toggling Editor Mode
  • Accessibility ?
Assignee: nobody → dothayer
Status: NEW → ASSIGNED

So, here's where this is currently at. I have a virtualized list up and running. It supports variable sized elements, and it pins the scrolling to the bottom if we were already there. The performance is night and day compared to the existing console - this is a profile of it handling an influx of 100000 messages: https://share.firefox.dev/3uorXY1. For context, the same amount of messages on today's firefox will make the browser completely unresponsive for several minutes.

So I'm fairly pleased with the result. However, there's a few things that still need to be sorted out:

  1. Filtering of messages. I just need to dig into this a bit more. Filtering works when we're scrolled up to the top of the output list, but it really doesn't like it when we're already at the bottom. I'm not entirely sure what that's about but it looks like it will probably just involve a simple tweak.
  2. Holding onto the expanded/collapsed state of messages after they're scrolled out of existence. This one is a bit more complicated - basically once we remove a list entry, we lose all the UI state associated with it. I'm not sure of the right path forward here, and I need to dig a little bit more into how that all fits together.

Once all of these are resolved, though, there's still more that we'll need to sort out to get the performance of this where I would like it to be (AKA, faster than Chrome's console.) We spend a lot of time in functions like getRepeatId and limitTopLevelMessageCount. There's a lot of complexity in the reducer side of things, and miscellaneous things show up all over there. I don't really know that Redux is a good fit for such a heavy workload. But I think that's out of the scope of this bug.

So, my current plan is just to fix points 1 and 2 above and put up a patch for this, and then start doing more exploration work on the broader performance picture.

I've fixed the filtering of messages - it did turn out to just be a minor bug. I haven't started working on persisting the expand/collapse state of messages yet, because the performance of this was still not what I wanted it to be. The big problem was synchronous style/layout flushes from all of the getBoundingClientRect calls and friends. So I burned those away by using the chrome-only promiseDocumentFlushed (though I'll need to sort out permissions on that, as it's stricter in its requirements than I think it needs to be?) - but then I was left still needing synchronous flushes in order to get it to correctly pin the scrolling to the bottom. I found this post by Ryan Hunt from a while back, which actually mentions nchevobbe, but I wasn't able to get the technique to work with the virtualized list. I'm not quite sure why it wouldn't work. I was able to get something somewhat working using Element.mozScrollSnap() though, and the result was much more performant (profile). I think I'm going to ask around among layout people if there's a better way to pin the scrolling to the bottom, because the performance wins of not having to manage this in JS seem pretty clear.

Whiteboard: dt-console-perf-2022

This is just some clean up - looks like this was once used to more dynamically
map strings to types, but now all of the mappings are explicitly accessed via
string literals.

Attachment #9262679 - Attachment description: WIP: Bug 1753177 - Use a virtualized list for webconsole output → Bug 1753177 - Use a virtualized list for webconsole output r?nchevobbe

Depends on D138610

Duplicate of this bug: 1411937
Duplicate of this bug: 1659773
Depends on: 1761082

This is a naive implementation where we re-create a ConsoleOutput
element, reusing the existing store, and passing a specific prop
so all the messages would be rendered.

Depends on D138031.

Depends on: 1763459
Pushed by dothayer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/012bf7c9090b
Clean up unnecessary componentMap in MessageContainer.js r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/fd0a28873bd1
Use a virtualized list for webconsole output r=nchevobbe
Regressions: 1764001
Summary: Investigate using a virtualized list for the webconsole → Use a virtualized list for the webconsole
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch

Comment on attachment 9269623 [details]
Bug 1753177 - [devtools] Make copy messages action compatible with virtualized console. r=dthayer.

Revision D142189 was moved to bug 1764236. Setting attachment 9269623 [details] to obsolete.

Attachment #9269623 - Attachment is obsolete: true
Regressions: 1766224
You need to log in before you can comment on or make changes to this bug.