This is an automated email from the ASF dual-hosted git repository.
bbovenzi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 124e0858b59 Memoize visible-items build in useLogGroups to fix scroll
perf (#73252)
124e0858b59 is described below
commit 124e0858b59a785c761cbb0b75dd59e126a880b9
Author: udsy19 <[email protected]>
AuthorDate: Mon Sep 21 11:21:37 2026 -0400
Memoize visible-items build in useLogGroups to fix scroll perf (#73252)
react-virtual force-rerenders TaskLogContent synchronously
(flushSync) on every native scroll event whose visible range
changes. useLogGroups rebuilt its entire visibleItems /
originalToVisibleIndex / lineNumberToVisibleIndex index with an
unmemoized O(total log lines) loop on every one of those renders,
so scroll cost scaled with total log size instead of the number
of rows actually on screen -- the mechanism behind #55173
("Viewing large task logs get slower to scroll as they get
larger").
Wrap the group-header index and the visible-items build in
useMemo, keyed on the values they actually depend on
(parsedLogs, expandedGroups, groupParentMap) so they only
recompute when the log content or expand state actually changes.
Generated-by: Claude Code
Signed-off-by: Udaya Tejas <[email protected]>
---
.../pages/TaskInstance/Logs/useLogGroups.test.tsx | 20 ++++
.../src/pages/TaskInstance/Logs/useLogGroups.tsx | 102 ++++++++++++---------
2 files changed, 81 insertions(+), 41 deletions(-)
diff --git
a/airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/useLogGroups.test.tsx
b/airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/useLogGroups.test.tsx
index 3b5af89a118..5b8a4b18acb 100644
---
a/airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/useLogGroups.test.tsx
+++
b/airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/useLogGroups.test.tsx
@@ -47,4 +47,24 @@ describe("useLogGroups", () => {
expect([...result.current.lineNumberToVisibleIndex]).toStrictEqual([[3,
2]]);
});
+
+ it("does not rebuild the visible-items index on a re-render that changes
neither parsedLogs nor expand state", () => {
+ // react-virtual force-rerenders TaskLogContent on every scroll event via
+ // flushSync, even though parsedLogs and expandedGroups are unchanged.
+ // Without memoization this hook rebuilds visibleItems/*ToVisibleIndex from
+ // scratch on every one of those renders, an O(total log lines) cost paid
+ // on every scroll tick regardless of how many rows are actually visible.
+ const { rerender, result } = renderHook(
+ ({ currentParsedLogs }) => useLogGroups({ expanded: true, parsedLogs:
currentParsedLogs }),
+ { initialProps: { currentParsedLogs: parsedLogs } },
+ );
+
+ const firstVisibleItems = result.current.visibleItems;
+ const firstLineNumberToVisibleIndex =
result.current.lineNumberToVisibleIndex;
+
+ rerender({ currentParsedLogs: parsedLogs });
+
+ expect(result.current.visibleItems).toBe(firstVisibleItems);
+
expect(result.current.lineNumberToVisibleIndex).toBe(firstLineNumberToVisibleIndex);
+ });
});
diff --git
a/airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/useLogGroups.tsx
b/airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/useLogGroups.tsx
index 12b290ffff9..1d0a68bcf65 100644
--- a/airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/useLogGroups.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/useLogGroups.tsx
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { useEffect, useState } from "react";
+import { useEffect, useMemo, useState } from "react";
import type { ParsedLogEntry } from "src/queries/useLogs";
@@ -38,16 +38,24 @@ export const useLogGroups = ({
parsedLogs: Array<ParsedLogEntry>;
searchMatchIndices?: Set<number>;
}) => {
- // Build parent map for nested visibility checks
- const groupHeaders = parsedLogs.filter(
- (entry): entry is { group: NonNullable<ParsedLogEntry["group"]> } &
ParsedLogEntry =>
- entry.group?.type === "header",
- );
- const allGroupIds = new Set(groupHeaders.map((entry) => entry.group.id));
- // eslint-disable-next-line react-hooks/exhaustive-deps -- React Compiler
auto-memoizes this
- const groupParentMap = new Map<number, number | undefined>(
- groupHeaders.map((entry) => [entry.group.id, entry.group.parentId]),
- );
+ // Build parent map for nested visibility checks. Memoized explicitly rather
+ // than relying on the compiler: react-virtual force-rerenders the consumer
+ // on every scroll event (`flushSync` in its React adapter), and without this
+ // memo the O(n) rebuild below re-ran on every one of those renders — cost
+ // scaling with total log size, not the virtualized/visible row count.
+ const { allGroupIds, groupParentMap } = useMemo(() => {
+ const groupHeaders = parsedLogs.filter(
+ (entry): entry is { group: NonNullable<ParsedLogEntry["group"]> } &
ParsedLogEntry =>
+ entry.group?.type === "header",
+ );
+
+ return {
+ allGroupIds: new Set(groupHeaders.map((entry) => entry.group.id)),
+ groupParentMap: new Map<number, number | undefined>(
+ groupHeaders.map((entry) => [entry.group.id, entry.group.parentId]),
+ ),
+ };
+ }, [parsedLogs]);
const [expandedGroups, setExpandedGroups] = useState<Set<number>>(() =>
expanded ? new Set(allGroupIds) : new Set<number>(),
@@ -78,45 +86,57 @@ export const useLogGroups = ({
});
};
- // Check if all ancestors of a group are expanded
- const isGroupAncestryExpanded = (groupId: number): boolean => {
- const parentId = groupParentMap.get(groupId);
-
- if (parentId === undefined) {
- return true;
- }
+ // Build visible items list with index mapping. Memoized for the same reason
+ // as groupParentMap above: this loop runs over every log line, and without
+ // memoization it re-ran on every react-virtual-forced re-render (i.e. on
+ // every scroll event), with cost scaling with total log size rather than
+ // the number of rows actually on screen.
+ const { lineNumberToVisibleIndex, originalToVisibleIndex, visibleItems } =
useMemo(() => {
+ // Check if all ancestors of a group are expanded
+ const isGroupAncestryExpanded = (groupId: number): boolean => {
+ const parentId = groupParentMap.get(groupId);
+
+ if (parentId === undefined) {
+ return true;
+ }
- return expandedGroups.has(parentId) && isGroupAncestryExpanded(parentId);
- };
+ return expandedGroups.has(parentId) && isGroupAncestryExpanded(parentId);
+ };
- const isEntryVisible = (entry: ParsedLogEntry): boolean => {
- if (!entry.group) {
- return true;
- }
+ const isEntryVisible = (entry: ParsedLogEntry): boolean => {
+ if (!entry.group) {
+ return true;
+ }
- if (entry.group.type === "header") {
- return isGroupAncestryExpanded(entry.group.id);
- }
+ if (entry.group.type === "header") {
+ return isGroupAncestryExpanded(entry.group.id);
+ }
- return expandedGroups.has(entry.group.id) &&
isGroupAncestryExpanded(entry.group.id);
- };
+ return expandedGroups.has(entry.group.id) &&
isGroupAncestryExpanded(entry.group.id);
+ };
- // Build visible items list with index mapping
- const visibleItems: Array<VisibleItem> = [];
- const originalToVisibleIndex = new Map<number, number>();
- const lineNumberToVisibleIndex = new Map<number, number>();
+ const items: Array<VisibleItem> = [];
+ const originalToVisible = new Map<number, number>();
+ const lineNumberToVisible = new Map<number, number>();
- for (let idx = 0; idx < parsedLogs.length; idx += 1) {
- const entry = parsedLogs[idx];
+ for (let idx = 0; idx < parsedLogs.length; idx += 1) {
+ const entry = parsedLogs[idx];
- if (entry && isEntryVisible(entry)) {
- originalToVisibleIndex.set(idx, visibleItems.length);
- if (entry.lineNumber !== undefined) {
- lineNumberToVisibleIndex.set(entry.lineNumber, visibleItems.length);
+ if (entry && isEntryVisible(entry)) {
+ originalToVisible.set(idx, items.length);
+ if (entry.lineNumber !== undefined) {
+ lineNumberToVisible.set(entry.lineNumber, items.length);
+ }
+ items.push({ entry, originalIndex: idx });
}
- visibleItems.push({ entry, originalIndex: idx });
}
- }
+
+ return {
+ lineNumberToVisibleIndex: lineNumberToVisible,
+ originalToVisibleIndex: originalToVisible,
+ visibleItems: items,
+ };
+ }, [expandedGroups, groupParentMap, parsedLogs]);
// Map search match indices from original to visible indices
const visibleSearchMatchIndices = searchMatchIndices
