Problem/Motivation

The Expand All handler awaits ensureRendered() for every comment id in sequence. On a 216-comment thread it fired ~15 requests and rendered only 139/216 cards after 20 seconds, and took tens of seconds to finish. The thread's main way to reveal the full discussion is effectively unusable at scale, and the UI stays busy for the whole run.

Steps to reproduce

  • Open a node with 200+ comments under ACT.
  • Click Expand all.
  • Observe the thread render in a long serial chain, reaching only a fraction of the comments in many seconds.

Proposed resolution

  • Batch deliveries through the existing delivery endpoint (up to 50 ids per request).
  • Parallelize the batches with a bounded concurrency limit instead of awaiting each comment.
  • Mark the Expand All button busy/disabled while loading, and refresh the total as batches complete.

Remaining tasks

  • Verify the full 216-comment thread loads in a bounded time on a real thread.

User interface changes

Expand All renders busy/disabled while loading and reveals the full thread in bounded batches rather than freezing for tens of seconds.

API changes

None beyond the bounded batch delivery in #3622839: On-demand fragment endpoints rebuild the full comment index.

Data model changes

None anticipated.

Comments

freelock created an issue. See original summary.

freelock’s picture

freelock
Location Seattle
commented
Issue summary: View changes
freelock’s picture

freelock
Location Seattle
commented
Related issues: +#3622839: On-demand fragment endpoints rebuild the full comment index

  • freelock committed e6f3b412 on 1.0.x 
    test: #3622840 Delay the first mock delivery batch to make the busy...

  • freelock committed ac3721c9 on 1.0.x 
    fix: #3622840 Expand All's concurrent delivery batches collide on a...

  • freelock committed f83fbdba on 1.0.x 
    fix: #3622840 Expand All does not scale on large threads
freelock’s picture

freelock
Location Seattle
commented
Status: Active » Needs work

Batch missing rows in progressive expand all.

Alpha3 follow-up: the progressive-loading case remains serial

The alpha3 patch adds bounded parallel delivery for already-rendered dirty rows, plus busy feedback. However, rows omitted by progressive loading still pass through serial ensureRendered() calls. This is the original large-thread scenario, not an unusual fallback.

A browser probe starting with 10 of 120 roots reached all rows through 37 serial context requests, maximum concurrency one, and no delivery requests. This measures request shape, not production latency. On sufficiently wide threads, that chain can also exhaust the anonymous context-request allowance.

Steps to reproduce

  1. Open a large discussion with only the initial progressive slice rendered.
  2. Click Expand All and inspect the context/delivery requests.
  3. Compare with the current batching test, whose rows already exist before expansion.

Remaining work and acceptance

  • Batch missing-row hydration as well as dirty-row refresh, preserving tree order and ancestor access rules.
  • Apply the same bounds to missing-row unread hydration.
  • Surface incomplete batches and allow retries without duplicate rows or lost drafts.
  • Test a genuinely partial 200+ comment tree, including failures and actual flood-control behavior.

Coordinate with the new sibling-position issue so newly delivered rows retain correct accessibility metadata.

  • freelock committed 0f9a9433 on 1.0.x 
    fix: #3622840 Expand All does not scale on large threads

  • freelock committed c1103528 on 1.0.x 
    fix: #3622840 Expand All does not scale on large threads
freelock’s picture

freelock
Location Seattle
commented
Status: Needs work » Fixed

Fixed in Alpha 4.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.