perf(tree): Lazily initialize event buffer#27382
Conversation
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (87 lines, 2 files), I've queued these reviewers:
How this works
|
There was a problem hiding this comment.
Pull request overview
This PR optimizes simple-tree node event handling by lazily allocating the per-node kernel event buffer, reducing allocations and internal event wiring for nodes that never receive event subscriptions.
Changes:
- Lazily initialize
TreeNodeKernel’s internalKernelEventBufferon first access tokernel.events, and migrate the source on hydration only if the buffer exists. - Add runtime protection against subscribing on a disposed event buffer.
- Extend
treeNodeKerneltests to cover hydration with pre-registered listeners, disposed-node subscription behavior, and subscribe/unsubscribe behavior withinwithBufferedTreeEvents.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/dds/tree/src/test/simple-tree/core/treeNodeKernel.spec.ts | Adds regression tests covering lazy buffer allocation edge cases across hydration, disposal, and buffered scopes. |
| packages/dds/tree/src/simple-tree/core/treeNodeKernel.ts | Implements lazy KernelEventBuffer creation and conditional migration/disposal logic; adds an assertion guarding event registration on disposed buffers. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…FluidFramework into tree/lazy-event-buffer
| } | ||
|
|
||
| public get events(): Listenable<KernelEvents> { | ||
| assert(!this.disposed, "Cannot register events on a disposed node"); |
There was a problem hiding this comment.
Why doesn't this also use this.assertNotDisposed()?
Most nodes likely don't get any event subscriptions. Lazy initialization avoids unnecessary allocations and internal event registrations..