Skip to content

Commit 74ce348

Browse files
Optimize AnchorSet field change event buffering (microsoft#23908)
## Description AnchorSet field change event buffering could go O(N^2). This corrects that by precomputing a map.
1 parent 7baf59c commit 74ce348

File tree

1 file changed

+34
-24
lines changed

1 file changed

+34
-24
lines changed

packages/dds/tree/src/core/tree/anchorSet.ts

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import {
1818
brand,
1919
brandedSlot,
2020
fail,
21+
getOrAddEmptyToMap,
22+
getOrCreate,
2123
} from "../../util/index.js";
2224
import type { FieldKey } from "../schema-stored/index.js";
2325

@@ -711,15 +713,7 @@ export class AnchorSet implements AnchorLocator {
711713
/**
712714
* Events collected during the visit which get sent as a batch during "free".
713715
*/
714-
bufferedEvents: [] as {
715-
node: PathNode;
716-
event: keyof AnchorEvents;
717-
/**
718-
* The key for the impacted field, if the event is associated with a key.
719-
* Some events, such as afterDestroy, do not involve a key, and thus leave this undefined.
720-
*/
721-
changedField?: FieldKey;
722-
}[],
716+
bufferedEvents: [] as BufferedEvent[],
723717

724718
// 'currentDepth' and 'depthThresholdForSubtreeChanged' serve to keep track of when do we need to emit
725719
// subtreeChangedAfterBatch events.
@@ -751,25 +745,31 @@ export class AnchorSet implements AnchorLocator {
751745
node.removeRef();
752746
}
753747
this.anchorSet.activeVisitor = undefined;
754-
const alreadyEmitted = new Map<PathNode, string[]>();
755-
for (const { node, event } of this.bufferedEvents) {
756-
if (!alreadyEmitted.has(node)) {
757-
alreadyEmitted.set(node, []);
748+
749+
// Aggregate changedFields by node.
750+
const eventsByNode: Map<PathNode, Set<FieldKey>> = new Map();
751+
for (const { node, event, changedField } of this.bufferedEvents) {
752+
if (event === "childrenChangedAfterBatch") {
753+
const keys = getOrCreate(eventsByNode, node, () => new Set());
754+
keys.add(
755+
changedField ??
756+
fail("childrenChangedAfterBatch events should have a changedField"),
757+
);
758758
}
759-
const emittedEvents = alreadyEmitted.get(node);
760-
if (emittedEvents?.includes(event) ?? false) {
759+
}
760+
761+
const alreadyEmitted = new Map<PathNode, (keyof AnchorEvents)[]>();
762+
for (const { node, event } of this.bufferedEvents) {
763+
const emittedEvents = getOrAddEmptyToMap(alreadyEmitted, node);
764+
if (emittedEvents.includes(event)) {
761765
continue;
762766
}
763-
emittedEvents?.push(event);
767+
emittedEvents.push(event);
764768
if (event === "childrenChangedAfterBatch") {
765-
const fieldKeys: FieldKey[] = this.bufferedEvents
766-
.filter((e) => e.node === node && e.event === event)
767-
.map(
768-
(e) =>
769-
e.changedField ??
770-
fail("childrenChangedAfterBatch events should have a changedField"),
771-
);
772-
node.events.emit(event, { changedFields: new Set(fieldKeys) });
769+
const changedFields =
770+
eventsByNode.get(node) ??
771+
fail("childrenChangedAfterBatch events should have changedFields");
772+
node.events.emit(event, { changedFields });
773773
} else {
774774
node.events.emit(event);
775775
}
@@ -1194,3 +1194,13 @@ function binaryFind(sorted: readonly PathNode[], index: number): PathNode | unde
11941194
}
11951195
return undefined; // If we reach here, target is not in array (or array was not sorted)
11961196
}
1197+
1198+
interface BufferedEvent {
1199+
node: PathNode;
1200+
event: keyof AnchorEvents;
1201+
/**
1202+
* The key for the impacted field, if the event is associated with a key.
1203+
* Some events, such as afterDestroy, do not involve a key, and thus leave this undefined.
1204+
*/
1205+
changedField?: FieldKey;
1206+
}

0 commit comments

Comments
 (0)