DEV Community

Cover image for An 80-second browser freeze in Zulip's Manage channels, fixed by killing quadratic DOM work
Pentine Pejay
Pentine Pejay Subscriber

Posted on

An 80-second browser freeze in Zulip's Manage channels, fixed by killing quadratic DOM work

Summer Bug Smash: Clear the Lineup šŸ›šŸ›¹

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

Project Overview

Zulip is an open-source team chat application: a Django backend and a TypeScript web client built on jQuery. The "Manage channels" overlay lists every channel you can see, with live search, sort, and filter. Each keystroke or filter change re-sorts the list in place.

Zulip

Bug Fix or Performance Improvement

I fixed issue #39755. On a server with about 331 channels, opening Manage channels (or the #channels/new view, or the All tab) makes the tab unresponsive for roughly 80 seconds. Chrome shows the "Page Unresponsive" dialog. The reporter's Chrome CPU profile pinned about 90% of the freeze on two DOM operations, removeChild and querySelectorAll, triggered through the bundled jQuery.

The code is redraw_left_panel() in web/src/stream_settings_ui.ts, which sorts the list by moving DOM rows into their new order. The re-append loop looked like this:

for (const stream_id of all_stream_ids) {
    const $widget = widgets.get(stream_id);
    scroll_util
        .get_content_element($("#channels_overlay_container .streams-list")) // re-queried every iteration
        .append($widget);
}
Enter fullscreen mode Exit fullscreen mode

The problem is the $("#channels_overlay_container .streams-list") inside the loop. That is a document-wide querySelectorAll, run once per channel. A querySelectorAll forces the browser to flush pending layout, so every one of the N appends triggers a full re-layout of the list. Detaching and re-appending N rows with a forced reflow each time is O(n²). With a few hundred rows, that is the 80-second freeze, and it matches the profile exactly: removeChild from the per-row detach, querySelectorAll from the per-row re-query.

Code

PR: https://github.com/kiprutopentine/zulip/pull/1

The fix, in redraw_left_panel():

// Query the content element once, before the loop.
const $content_element = scroll_util.get_content_element(
    $("#channels_overlay_container .streams-list"),
);

// ...scan .stream-row a single time, set classes, detach rows...

// Re-append each row into the CACHED element. The original re-queried the
// content element here on every iteration; that document-wide querySelectorAll
// forces a synchronous layout flush of the pending append, so each row paid a
// full reflow. Caching it removes the forced layout, and one append form keeps
// the change lint- and mock-compatible with the existing test.
for (const stream_id of all_stream_ids) {
    const $widget = widgets.get(stream_id);
    assert($widget !== undefined);
    $content_element.append($widget);
}
Enter fullscreen mode Exit fullscreen mode

Three changes: query the content element once (this removes the per-row querySelectorAll and the forced synchronous layout it caused), scan .stream-row a single time instead of twice, and re-append each row into that cached element instead of re-looking it up every iteration. The ordering, the notdisplayed classes, and the function's return value are all unchanged, so behavior is identical, only the DOM work drops from O(n²) to O(n).

My Improvements

I kept the change inside the existing function and its jQuery idiom rather than rewriting the overlay. That was deliberate: Zulip's frontend tests run against a jQuery mock (zjquery) whose elements are not real DOM nodes, so a raw DocumentFragment reorder would pass in the browser but break the test suite. Staying on .detach() / .append() keeps the existing redraw_left_panel test valid, which already asserts the sorted order across search, sort, and filter cases.

  • Removed the per-iteration content-element lookup, the O(n²) driver: a document-wide querySelectorAll that forced a synchronous layout flush of the pending append on every row.
  • Collapsed two .stream-row scans into one.
  • Kept the plain .append($row) form, so the change is behavior- and lint-identical to the original and compatible with Zulip's zjquery test mock (a raw DocumentFragment reorder would pass in a browser but break the mock, whose elements are not real DOM nodes).

Verification: I provisioned a full Zulip dev environment on a throwaway cloud VM and ran Zulip's own tooling against the fix:

  • ./tools/test-js-with-node stream_settings_ui → test: redraw_left_panel passes ("Test(s) passed. SUCCESS!").
  • ./tools/run-tsc clean.
  • ./tools/lint --only=eslint,prettier web/src/stream_settings_ui.ts clean.

The change is behavior-preserving (same order, notdisplayed classes, return value), so that existing test, which pins ordering across search, sort, and filter cases, passes unchanged.

Best Use of Sentry

This is a performance bug with zero exceptions, so error monitoring alone would never see it. I used Sentry's performance tooling to make the cost visible and to prove the fix.

I built a benchmark harness (@sentry/browser via the loader script) that rebuilds the exact redraw both ways against N real .stream-row nodes and wraps each run in a Sentry span named manage_channels.redraw_left_panel (op: ui.render):

  • OLD detaches each row and re-appends one at a time, re-querying the content element every iteration (forcing a synchronous layout per row): O(n²).
  • NEW queries the content element once and re-inserts in a single batch: O(n).

Measured in the harness, which is deliberately minimal (plain rows) so the numbers isolate the algorithm itself rather than Zulip's full per-row render cost:

channels OLD (buggy) NEW (fixed)
331 115.8 ms 3.7 ms
1000 477.4 ms 7.3 ms
2000 2340.4 ms 15.1 ms

OLD grows super-linearly, close to quadratic at the larger sizes: doubling the list from 1000 to 2000 channels roughly quintuples the time. NEW scales linearly and stays in the low milliseconds. At 2000 channels the redraw drops from 2.3 seconds to 15 ms, about 155x. In the production overlay each row is far heavier (real templates, jQuery, the scroll container), so the same pattern is what turns "a few hundred channels" into the reporter's ~80-second freeze instead of a fraction of a second. The harness proves the mechanism and the fix; the 80 seconds is the real-world symptom of it.

Performance / tracing: the OLD run does not just take longer, it blocks the browser's main thread. Sentry's tracing recorded it as a ui.long-animation-frame — Main UI thread blocked span (about 1.3 s in this trace), exactly the kind of long task that makes the tab unresponsive. The NEW run produces no such block.

trace

Each run is tagged with its channels count and its kind (old/new), so the regression is visible inside Sentry, not just in the console. Filtering the manage_channels.redraw_left_panel transactions shows the fixed runs sitting in the low milliseconds (4–30 ms) while the buggy runs climb with the channel count, from ~100 ms up past 2 seconds, and beyond 17 seconds on a larger list. Sentry is not just recording one slow event; it shows the shape of the algorithm, and that the fix flattens it.

old and new

Session Replay: the replay of the OLD run shows the list locked and unresponsive; the NEW run is instant.

reply

Three Sentry surfaces carried this fix: tracing timed each redraw as a ui.render transaction and plotted the cost curve as the channel count grew, its long-animation-frame detection flagged the blocked main thread on its own, and Session Replay captured the freeze the way a user feels it. Run it yourself, the harness is public at https://github.com/kiprutopentine/sentry

Top comments (0)