udsy19 opened a new pull request, #73252:
URL: https://github.com/apache/airflow/pull/73252
## Summary
closes: #55173
`TaskLogContent` re-renders on every scroll event
(`@tanstack/react-virtual`'s
scroll handler calls `flushSync(rerender)` synchronously whenever the visible
row range changes), and `useLogGroups` rebuilt its entire `visibleItems` /
`originalToVisibleIndex` / `lineNumberToVisibleIndex` index with an
unmemoized loop over the **whole** `parsedLogs` array on every one of those
renders — cost scaling with total log size, not with the handful of rows
actually on screen. That is the mechanism behind #55173: scrolling a large
task log gets progressively slower as the log grows, because every scroll
tick pays for a full rebuild of the whole log's visibility index, not just
the virtualized viewport.
Who reaches this: any user viewing task logs in the Airflow 3 UI
(`/dags/{dag_id}/runs/{run_id}/tasks/{task_id}`, Logs tab) — the standard,
documented way to read task output. Triggered by scrolling (wheel/trackpad)
a log of non-trivial size; every native `scroll` event on the log container
forces this rebuild.
Impact: perf-regression
## Fix
Wrap the group-header index and the visible-items build in `useMemo`, keyed
on what they actually depend on (`parsedLogs`, `expandedGroups`,
`groupParentMap`), so they only recompute when the log content or the
expand/collapse state actually changes, not on every scroll-triggered
re-render.
## Measured before/after
Isolated `useLogGroups` with `renderHook`, forcing a re-render whose props
are identical to the previous render (the same shape of update
`react-virtual` triggers on every scroll event where `parsedLogs` and
`expandedGroups` are unchanged). Measured with `performance.now()`, median
of 15 repeat re-renders per log size, in `vitest` (`happy-dom`).
Before: 43.571 ms / After: 0.118 ms, on the largest log size measured
(see the full size sweep below).
```
before (baseline, unpatched): 1,000 lines: 0.368ms 10,000 lines: 2.270ms
100,000 lines: 43.571ms
after (with this patch): 1,000 lines: 0.128ms 10,000 lines: 0.057ms
100,000 lines: 0.118ms
```
Before the fix, cost scales with total log size (~120x from 1k to 100k
lines) and already exceeds a 16ms/60fps frame budget at 100k — on a hot path
that runs on every scroll event, not once per log load. After the fix, cost
is flat, since the rebuild only reruns when its inputs actually change.
## Negative control
Added `useLogGroups.test.tsx` › *"does not rebuild the visible-items index
on a re-render that changes neither parsedLogs nor expand state"*: renders
the hook, captures `visibleItems`/`lineNumberToVisibleIndex`, re-renders
with the identical `parsedLogs` reference and no state change, and asserts
referential equality (`toBe`) — true only if the memoization actually held.
Test fails without the fix, passes with it:
```
without the fix (useLogGroups.tsx reverted to main): 1 failed | 2 skipped (3)
AssertionError: expected [ { …(2) }, …(4) ] to be [ { …(2) }, …(4) ] //
Object.is equality
(same content, different object identity — the array/map is rebuilt every
render)
with the fix: 3 passed (3)
```
## Was generative AI tooling used to co-author this PR?
- [x] Yes (please specify the tool below)
Claude Code
Generated-by: Claude Code following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
Signed-off-by: Udaya Tejas <[email protected]>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
