diff --git a/CHANGELOG.md b/CHANGELOG.md index 8492bc4485..03ee7add1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -340,7 +340,8 @@ Breaking changes in this release: - Improved adaptive cards rendering in copilot variant, in PR [#5682](https://github.com/microsoft/BotFramework-WebChat/pull/5682), by [@OEvgeny](https://github.com/OEvgeny) - Bumped to [`botframework-directlinejs@0.15.8`](https://www.npmjs.com/package/botframework-directlinejs/v/0.15.8) to include support for the new `streaming` property, by [@pranavjoshi001](https://github.com/pranavjoshi001), in PR [#5686](https://github.com/microsoft/BotFramework-WebChat/pull/5686) - Removed unused deps `simple-git`, by [@compulim](https://github.com/compulim), in PR [#5786](https://github.com/microsoft/BotFramework-WebChat/pull/5786) -- Improved `ActivityKeyerComposer` performance for append scenarios by adding an incremental fast path that only processes newly-appended activities, in PR [#5790](https://github.com/microsoft/BotFramework-WebChat/pull/5790), by [@OEvgeny](https://github.com/OEvgeny) +- Improved `ActivityKeyerComposer` performance for append scenarios by adding an incremental fast path that only processes newly-appended activities, in PR [#5790](https://github.com/microsoft/BotFramework-WebChat/pull/5790), in PR [#5797](https://github.com/microsoft/BotFramework-WebChat/pull/5797), by [@OEvgeny](https://github.com/OEvgeny) + - Added frozen window optimization to limit reference comparisons to the last 1,000 activities with deferred verification of the frozen portion, in PR [#5797](https://github.com/microsoft/BotFramework-WebChat/pull/5797), by [@OEvgeny](https://github.com/OEvgeny) ### Deprecated diff --git a/packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx b/packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx index 93287acc36..a7be34d749 100644 --- a/packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx +++ b/packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx @@ -1,8 +1,9 @@ import { getActivityLivestreamingMetadata, type WebChatActivity } from 'botframework-webchat-core'; -import React, { useCallback, useMemo, useRef, type ReactNode } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef, type ReactNode } from 'react'; import reduceIterable from '../../hooks/private/reduceIterable'; import useActivities from '../../hooks/useActivities'; +import usePonyfill from '../Ponyfill/usePonyfill'; import type { ActivityKeyerContextType } from './private/Context'; import ActivityKeyerContext from './private/Context'; import getActivityId from './private/getActivityId'; @@ -17,6 +18,12 @@ type ActivityToKeyMap = Map; type ClientActivityIdToKeyMap = Map; type KeyToActivitiesMap = Map; +/** After this many ms of no activity changes, verify that the frozen portion was not modified. */ +const FROZEN_CHECK_TIMEOUT = 10_000; + +/** Only the last N activities are compared reference-by-reference on each render. */ +const MUTABLE_ACTIVITY_WINDOW = 1_000; + /** * React context composer component to assign a perma-key to every activity. * This will support both `useGetActivityByKey` and `useGetKeyByActivity` custom hooks. @@ -32,6 +39,7 @@ type KeyToActivitiesMap = Map; * Local key are only persisted in memory. On refresh, they will be a new random key. */ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | undefined }>) => { + const [{ cancelIdleCallback, clearTimeout, requestIdleCallback, setTimeout }] = usePonyfill(); const existingContext = useActivityKeyerContext(false); if (existingContext) { @@ -52,14 +60,25 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u const prevActivityKeysStateRef = useRef( Object.freeze([Object.freeze([])]) as readonly [readonly string[]] ); + const pendingFrozenCheckRef = useRef< + | { + readonly current: readonly WebChatActivity[]; + readonly frozenBoundary: number; + readonly prev: readonly WebChatActivity[]; + } + | undefined + >(); + const warnedPositionsRef = useRef>(new Set()); // Incremental keying: the fast path only processes newly-appended activities (O(delta) per render) // instead of re-iterating all activities (O(n) per render, O(n²) total for n streaming pushes). const activityKeysState = useMemo(() => { const prevActivities = prevActivitiesRef.current; - // Detect how many leading activities are identical (same reference) to the previous render. - let commonPrefixLength = 0; + // Only the last MUTABLE_ACTIVITY_WINDOW activities are compared each render. + // Activities before the frozen boundary are assumed unchanged — O(1) instead of O(n). + const frozenBoundary = Math.max(0, Math.min(prevActivities.length, activities.length) - MUTABLE_ACTIVITY_WINDOW); + let commonPrefixLength = frozenBoundary; const maxPrefix = Math.min(prevActivities.length, activities.length); // eslint-disable-next-line security/detect-object-injection @@ -78,6 +97,12 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u return prevActivityKeysStateRef.current; } + // Schedule deferred verification of the frozen portion only now that we know: + // (1) the update is append-only and (2) there are actual content changes. + if (frozenBoundary) { + pendingFrozenCheckRef.current = Object.freeze({ current: activities, frozenBoundary, prev: prevActivities }); + } + const { current: activityIdToKeyMap } = activityIdToKeyMapRef; const { current: activityToKeyMap } = activityToKeyMapRef; const { current: clientActivityIdToKeyMap } = clientActivityIdToKeyMapRef; @@ -118,19 +143,15 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u prevActivitiesRef.current = activities; - if (newKeys.length) { - const nextKeys = Object.freeze([...prevActivityKeysStateRef.current[0], ...newKeys]); - const result = Object.freeze([nextKeys]) as readonly [readonly string[]]; - - prevActivityKeysStateRef.current = result; - - return result; + if (!newKeys.length) { + // New activities might be added to existing keys — no new keys, but the keyToActivitiesMap + // was mutated. Return a new tuple reference so context consumers re-render and see the + // updated activities-per-key via getActivitiesByKey. + return Object.freeze([prevActivityKeysStateRef.current[0]]) as readonly [readonly string[]]; } - // New activities were added to existing keys — no new keys, but the keyToActivitiesMap - // was mutated. Return a new tuple reference so context consumers re-render and see the - // updated activities-per-key via getActivitiesByKey. - const result = Object.freeze([prevActivityKeysStateRef.current[0]]) as readonly [readonly string[]]; + const nextKeys = Object.freeze([...prevActivityKeysStateRef.current[0], ...newKeys]); + const result = Object.freeze([nextKeys]) as readonly [readonly string[]]; prevActivityKeysStateRef.current = result; @@ -180,6 +201,10 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u keyToActivitiesMapRef.current = nextKeyToActivitiesMap; prevActivitiesRef.current = activities; + // Slow path did a full recalculation — no frozen check needed, reset warnings. + pendingFrozenCheckRef.current = undefined; + warnedPositionsRef.current.clear(); + const nextKeys = Object.freeze([...nextActivityKeys.values()]); const result = Object.freeze([nextKeys]) as readonly [readonly string[]]; @@ -192,10 +217,53 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u activityToKeyMapRef, clientActivityIdToKeyMapRef, keyToActivitiesMapRef, + pendingFrozenCheckRef, prevActivitiesRef, - prevActivityKeysStateRef + prevActivityKeysStateRef, + warnedPositionsRef ]); + // Deferred verification: after FROZEN_CHECK_TIMEOUT of quiet, validate that activities + // inside the frozen portion have not actually changed. Warn once per position if they did. + // Uses requestIdleCallback inside the timeout to avoid contending with the first post-stream repaint. + useEffect(() => { + const pending = pendingFrozenCheckRef.current; + + if (!pending) { + return; + } + + let idleHandle: ReturnType> | undefined; + + const runCheck = () => { + const { current: currentActivities, frozenBoundary, prev: prevFrozenActivities } = pending; + + for (let i = 0; i < frozenBoundary; i++) { + // eslint-disable-next-line security/detect-object-injection + if (prevFrozenActivities[i] !== currentActivities[i] && !warnedPositionsRef.current.has(i)) { + warnedPositionsRef.current.add(i); + + console.warn( + `botframework-webchat internal: change in activity at position ${i} was not applied because it is outside the mutable window of ${MUTABLE_ACTIVITY_WINDOW}.` + ); + } + } + }; + + const timer = setTimeout(() => { + if (requestIdleCallback) { + idleHandle = requestIdleCallback(runCheck); + } else { + runCheck(); + } + }, FROZEN_CHECK_TIMEOUT); + + return () => { + clearTimeout(timer); + idleHandle !== undefined && cancelIdleCallback?.(idleHandle); + }; + }, [activities, cancelIdleCallback, clearTimeout, requestIdleCallback, setTimeout]); + const getActivitiesByKey: (key?: string | undefined) => readonly WebChatActivity[] | undefined = useCallback( (key?: string | undefined): readonly WebChatActivity[] | undefined => key && keyToActivitiesMapRef.current.get(key), [keyToActivitiesMapRef]