perf(tree): return bound off() from KernelEventBuffer.on

[Josmithr]

Overview

Replace the per-subscription () => this.off(eventName, listener) arrow returned by KernelEventBuffer.on with this.off.bind(this, eventName, listener).

A V8 JSBoundFunction stores (target, thisArg, ...boundArgs) in a fixed shape and is significantly more compact than a fresh arrow + Context pair, which captures the surrounding lexical scope. Functionally identical — both forms invoke this.off(eventName, listener) with no other state.

No public API surface change; internal-only.

[github-actions]

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (5 lines, 1 files), I've queued these reviewers:

• Correctness — logic errors, race conditions, lifecycle issues
• Security — vulnerabilities, secret exposure, injection
• API Compatibility — breaking changes, release tags, type design
• Performance — algorithmic regressions, memory leaks
• Testing — coverage gaps, hollow tests

How this works

• Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

• Tick Start review below to dispatch the review fleet.

• After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

• This comment updates as new commits land; your reviewer selections are preserved.

• Start review

[Copilot]

Pull request overview

Improves @fluidframework/tree internal event subscription memory/CPU overhead by returning a bound off() function from KernelEventBuffer.on() instead of allocating a new arrow-function closure per subscription.

Changes:

• Replace () => this.off(eventName, listener) with this.off.bind(this, eventName, listener) in KernelEventBuffer.on().
• Add an inline comment explaining the V8 motivation for preferring bound functions over closures.

[noencke]

TIL. Though, I'm still surprised this is anything more than a micro-op. Is this premature optimization? Or is the rationale that because it's the same amount of lines, we may as well just do it the faster way?

Do you think it's something we might employ elsewhere? If we do, we wouldn't want to repeat these same three lines of comments at every instance. We could make it a helper - that would add a micro-cost at registration time, but wouldn't add any additional cost at "off" time. Or, just leave it as is, but remove the comment and instead put a new entry in our coding best practices document that says to use bind when you only need this and no other state, as a micro-op?

This is fine as is, just trying to sniff out how this technique generalizes...

[Josmithr]

TIL. Though, I'm still surprised this is anything more than a micro-op. Is this premature optimization? Or is the rationale that because it's the same amount of lines, we may as well just do it the faster way?

Do you think it's something we might employ elsewhere? If we do, we wouldn't want to repeat these same three lines of comments at every instance. We could make it a helper - that would add a micro-cost at registration time, but wouldn't add any additional cost at "off" time. Or, just leave it as is, but remove the comment and instead put a new entry in our coding best practices document that says to use bind when you only need this and no other state, as a micro-op?

This is fine as is, just trying to sniff out how this technique generalizes...

This was something Claude identified. It ran its own perf test suite which seemed to show this change having a noticeable impact, given how hot this code path is. I omitted the results from the description because they referred to a perf suite that hasn't been checked in yet. I have a draft PR up for those here: #27372. Waiting for the PR build to pass before promoting it for review.