Skip to content

Commit 94c9e6f

Browse files
OEvgenyCopilot
andauthored
perf: reduce iterations in the fast path for the keyer composer updates (#5797)
* perf: reduce iterations in the fast path for the keyer composer updates * Changelog * perf: schedule pendingFrozenCheckRef only after confirming append-only with content changes Agent-Logs-Url: https://github.com/microsoft/BotFramework-WebChat/sessions/d07bf01e-33c1-440a-a28e-095207d61357 Co-authored-by: OEvgeny <2841858+OEvgeny@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: OEvgeny <2841858+OEvgeny@users.noreply.github.com>
1 parent a38e350 commit 94c9e6f

File tree

2 files changed

+85
-16
lines changed

2 files changed

+85
-16
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,8 @@ Breaking changes in this release:
341341
- Improved adaptive cards rendering in copilot variant, in PR [#5682](https://github.com/microsoft/BotFramework-WebChat/pull/5682), by [@OEvgeny](https://github.com/OEvgeny)
342342
- 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)
343343
- Removed unused deps `simple-git`, by [@compulim](https://github.com/compulim), in PR [#5786](https://github.com/microsoft/BotFramework-WebChat/pull/5786)
344-
- 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)
344+
- 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)
345+
- 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)
345346
- Bumped to [`adaptivecards@3.0.6`](https://www.npmjs.com/package/adaptivecards/v/3.0.6) in PR [#5800](https://github.com/microsoft/BotFramework-WebChat/pull/5800) by [@compulim](https://github.com/compulim)
346347

347348
### Deprecated

packages/api/src/providers/ActivityKeyer/ActivityKeyerComposer.tsx

Lines changed: 83 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { getActivityLivestreamingMetadata, type WebChatActivity } from 'botframework-webchat-core';
2-
import React, { useCallback, useMemo, useRef, type ReactNode } from 'react';
2+
import React, { useCallback, useEffect, useMemo, useRef, type ReactNode } from 'react';
33

44
import reduceIterable from '../../hooks/private/reduceIterable';
55
import useActivities from '../../hooks/useActivities';
6+
import usePonyfill from '../Ponyfill/usePonyfill';
67
import type { ActivityKeyerContextType } from './private/Context';
78
import ActivityKeyerContext from './private/Context';
89
import getActivityId from './private/getActivityId';
@@ -17,6 +18,12 @@ type ActivityToKeyMap = Map<WebChatActivity, string>;
1718
type ClientActivityIdToKeyMap = Map<string, string>;
1819
type KeyToActivitiesMap = Map<string, readonly WebChatActivity[]>;
1920

21+
/** After this many ms of no activity changes, verify that the frozen portion was not modified. */
22+
const FROZEN_CHECK_TIMEOUT = 10_000;
23+
24+
/** Only the last N activities are compared reference-by-reference on each render. */
25+
const MUTABLE_ACTIVITY_WINDOW = 1_000;
26+
2027
/**
2128
* React context composer component to assign a perma-key to every activity.
2229
* This will support both `useGetActivityByKey` and `useGetKeyByActivity` custom hooks.
@@ -32,6 +39,7 @@ type KeyToActivitiesMap = Map<string, readonly WebChatActivity[]>;
3239
* Local key are only persisted in memory. On refresh, they will be a new random key.
3340
*/
3441
const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | undefined }>) => {
42+
const [{ cancelIdleCallback, clearTimeout, requestIdleCallback, setTimeout }] = usePonyfill();
3543
const existingContext = useActivityKeyerContext(false);
3644

3745
if (existingContext) {
@@ -52,14 +60,25 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u
5260
const prevActivityKeysStateRef = useRef<readonly [readonly string[]]>(
5361
Object.freeze([Object.freeze([])]) as readonly [readonly string[]]
5462
);
63+
const pendingFrozenCheckRef = useRef<
64+
| {
65+
readonly current: readonly WebChatActivity[];
66+
readonly frozenBoundary: number;
67+
readonly prev: readonly WebChatActivity[];
68+
}
69+
| undefined
70+
>();
71+
const warnedPositionsRef = useRef<Set<number>>(new Set());
5572

5673
// Incremental keying: the fast path only processes newly-appended activities (O(delta) per render)
5774
// instead of re-iterating all activities (O(n) per render, O(n²) total for n streaming pushes).
5875
const activityKeysState = useMemo<readonly [readonly string[]]>(() => {
5976
const prevActivities = prevActivitiesRef.current;
6077

61-
// Detect how many leading activities are identical (same reference) to the previous render.
62-
let commonPrefixLength = 0;
78+
// Only the last MUTABLE_ACTIVITY_WINDOW activities are compared each render.
79+
// Activities before the frozen boundary are assumed unchanged — O(1) instead of O(n).
80+
const frozenBoundary = Math.max(0, Math.min(prevActivities.length, activities.length) - MUTABLE_ACTIVITY_WINDOW);
81+
let commonPrefixLength = frozenBoundary;
6382
const maxPrefix = Math.min(prevActivities.length, activities.length);
6483

6584
// eslint-disable-next-line security/detect-object-injection
@@ -78,6 +97,12 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u
7897
return prevActivityKeysStateRef.current;
7998
}
8099

100+
// Schedule deferred verification of the frozen portion only now that we know:
101+
// (1) the update is append-only and (2) there are actual content changes.
102+
if (frozenBoundary) {
103+
pendingFrozenCheckRef.current = Object.freeze({ current: activities, frozenBoundary, prev: prevActivities });
104+
}
105+
81106
const { current: activityIdToKeyMap } = activityIdToKeyMapRef;
82107
const { current: activityToKeyMap } = activityToKeyMapRef;
83108
const { current: clientActivityIdToKeyMap } = clientActivityIdToKeyMapRef;
@@ -118,19 +143,15 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u
118143

119144
prevActivitiesRef.current = activities;
120145

121-
if (newKeys.length) {
122-
const nextKeys = Object.freeze([...prevActivityKeysStateRef.current[0], ...newKeys]);
123-
const result = Object.freeze([nextKeys]) as readonly [readonly string[]];
124-
125-
prevActivityKeysStateRef.current = result;
126-
127-
return result;
146+
if (!newKeys.length) {
147+
// New activities might be added to existing keys — no new keys, but the keyToActivitiesMap
148+
// was mutated. Return a new tuple reference so context consumers re-render and see the
149+
// updated activities-per-key via getActivitiesByKey.
150+
return Object.freeze([prevActivityKeysStateRef.current[0]]) as readonly [readonly string[]];
128151
}
129152

130-
// New activities were added to existing keys — no new keys, but the keyToActivitiesMap
131-
// was mutated. Return a new tuple reference so context consumers re-render and see the
132-
// updated activities-per-key via getActivitiesByKey.
133-
const result = Object.freeze([prevActivityKeysStateRef.current[0]]) as readonly [readonly string[]];
153+
const nextKeys = Object.freeze([...prevActivityKeysStateRef.current[0], ...newKeys]);
154+
const result = Object.freeze([nextKeys]) as readonly [readonly string[]];
134155

135156
prevActivityKeysStateRef.current = result;
136157

@@ -180,6 +201,10 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u
180201
keyToActivitiesMapRef.current = nextKeyToActivitiesMap;
181202
prevActivitiesRef.current = activities;
182203

204+
// Slow path did a full recalculation — no frozen check needed, reset warnings.
205+
pendingFrozenCheckRef.current = undefined;
206+
warnedPositionsRef.current.clear();
207+
183208
const nextKeys = Object.freeze([...nextActivityKeys.values()]);
184209
const result = Object.freeze([nextKeys]) as readonly [readonly string[]];
185210

@@ -192,10 +217,53 @@ const ActivityKeyerComposer = ({ children }: Readonly<{ children?: ReactNode | u
192217
activityToKeyMapRef,
193218
clientActivityIdToKeyMapRef,
194219
keyToActivitiesMapRef,
220+
pendingFrozenCheckRef,
195221
prevActivitiesRef,
196-
prevActivityKeysStateRef
222+
prevActivityKeysStateRef,
223+
warnedPositionsRef
197224
]);
198225

226+
// Deferred verification: after FROZEN_CHECK_TIMEOUT of quiet, validate that activities
227+
// inside the frozen portion have not actually changed. Warn once per position if they did.
228+
// Uses requestIdleCallback inside the timeout to avoid contending with the first post-stream repaint.
229+
useEffect(() => {
230+
const pending = pendingFrozenCheckRef.current;
231+
232+
if (!pending) {
233+
return;
234+
}
235+
236+
let idleHandle: ReturnType<NonNullable<typeof requestIdleCallback>> | undefined;
237+
238+
const runCheck = () => {
239+
const { current: currentActivities, frozenBoundary, prev: prevFrozenActivities } = pending;
240+
241+
for (let i = 0; i < frozenBoundary; i++) {
242+
// eslint-disable-next-line security/detect-object-injection
243+
if (prevFrozenActivities[i] !== currentActivities[i] && !warnedPositionsRef.current.has(i)) {
244+
warnedPositionsRef.current.add(i);
245+
246+
console.warn(
247+
`botframework-webchat internal: change in activity at position ${i} was not applied because it is outside the mutable window of ${MUTABLE_ACTIVITY_WINDOW}.`
248+
);
249+
}
250+
}
251+
};
252+
253+
const timer = setTimeout(() => {
254+
if (requestIdleCallback) {
255+
idleHandle = requestIdleCallback(runCheck);
256+
} else {
257+
runCheck();
258+
}
259+
}, FROZEN_CHECK_TIMEOUT);
260+
261+
return () => {
262+
clearTimeout(timer);
263+
idleHandle !== undefined && cancelIdleCallback?.(idleHandle);
264+
};
265+
}, [activities, cancelIdleCallback, clearTimeout, requestIdleCallback, setTimeout]);
266+
199267
const getActivitiesByKey: (key?: string | undefined) => readonly WebChatActivity[] | undefined = useCallback(
200268
(key?: string | undefined): readonly WebChatActivity[] | undefined => key && keyToActivitiesMapRef.current.get(key),
201269
[keyToActivitiesMapRef]

0 commit comments

Comments
 (0)