Repository navigation
fix: Commands no longer run several times slower while the Unity Editor stays in the background on macOS - #3200
Merged
Conversation
macOS throttles a background Editor, and the fix holds an operating-system activity while a command runs. Ending the same native token twice touches released memory, so one registry owns the live tokens: each is ended by its handle or by the close before a domain reload, whichever comes first, and no activity starts after the close. A platform without the native entry points runs commands without an activity instead of failing them, and a failure while closing is logged so the other reload subscribers still run.
A request that reaches a throttled background Editor on macOS is served several times slower, so the router now holds an activity from the moment it dispatches a tool or internal bridge command until that call returns, throws or is canceled. The editor status answer stays outside the hold because it must answer while the main thread is stuck. The registry is created once per domain in the composition root and handed to every router the server factory builds; the test double moves to its own file so the router tests can watch the hold from inside a tool.
The registry now uses NSProcessInfo's beginActivityWithOptions:reason: with NSActivityUserInitiatedAllowingIdleSystemSleep, the smallest option measured to lift App Nap, and leaves idle system sleep alone. Begin runs inside its own autorelease pool because thread-pool threads have none, and retains the token before the pool drains so the activity outlives the call. Other platforms keep the inert API, and the declarations resolve only when called, so no conditional compilation is needed.
A mutation test that let a hold end its token after the registry had already closed brought the Editor down with SIGTRAP inside endActivity:. The comments on the two places that end a token now say that a second end is a crash rather than an exception, so the remove-before-End order is not mistaken for defensive tidiness.
The decision has alternatives that look simpler (holding for as long as the server runs, asking users to disable App Nap with defaults, other activity options), so the ADR keeps the measurements that ruled them out, what the hold does not cover, and when to reopen the question.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (9)
📒 Files selected for processing (19)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change adds command-scoped macOS process activities for routed commands. It tracks and releases activity tokens, closes live activities before domain reload, and uses inert activity behavior on other platforms. Status requests do not hold an activity. ChangesEditor execution activity
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Caller
participant UnityCliLoopExecutionRouter
participant EditorExecutionActivity
participant Tool
Caller->>UnityCliLoopExecutionRouter: Route non-status request
UnityCliLoopExecutionRouter->>EditorExecutionActivity: Hold activity
UnityCliLoopExecutionRouter->>Tool: Dispatch request
Tool-->>UnityCliLoopExecutionRouter: Return result or exception
UnityCliLoopExecutionRouter->>EditorExecutionActivity: Dispose hold
UnityCliLoopExecutionRouter-->>Caller: Return response or propagate exception
Merge Risk: 🔵 Low · up to The change keeps a macOS activity alive while a command runs, and other platforms are unaffected. The native macOS calls are the only residual risk, and the author reports they passed testing. This is mergeable with minor owner awareness. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change is confined to keeping the addressed macOS Editor responsive during commands. Existing command permissions remain in place, and cleanup is centralized. Limited uncertainty remains around native cleanup failures and commands that never finish; no new permission bypass was established. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 42.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 61 functions across 18 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…pletes The old tool test completed synchronously, so the whole call finished before ExecuteAsync returned, and a router that released the hold before awaiting the tool still passed it. The test now uses a tool whose task stays pending, checks the activity is still held while it is pending, and completes it in finally so a failed assertion leaves nothing behind.
hatayama
marked this pull request as ready for review
October 7, 2026 01:12
This was referenced Oct 7, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
uloopcommand it serves runs several times slower. The Editor now holds a macOS activity while it processes a command, so a background Editor serves commands as fast as one in front.User Impact
Changes
EditorExecutionActivity(new,Infrastructure/ExecutionActivity/) is the only entry point.Hold()begins an activity and returns a handle whoseDispose()ends it.ReleaseAllAndClose(), subscribed tobeforeAssemblyReload, ends every live activity and starts no more. A token leaves the set of live tokens beforeEndis called, so each token is ended exactly once, by whichever comes first.MacProcessActivityApi(new) callsNSProcessInfo beginActivityWithOptions:reason:withNSActivityUserInitiatedAllowingIdleSystemSleepthrough libobjc. It retains the autoreleased token before its own autorelease pool drains, and releases it afterendActivity:. There is no#if: the declarations resolve only when called. Other platforms useInertProcessActivityApi, which starts nothing.UnityCliLoopExecutionRouterwraps internal bridge commands and tools inusing (Hold()). Theget-editor-statusearly return is unchanged, so it still answers while the main thread is stuck.UnityCliLoopBridgeServerInstanceFactory.Input space
BegingivesOperationCanceledExceptionpropagates; ended onceget-editor-statusBeginis not calledDisposedoes nothingBeginIntPtr.ZeroEndis not calledDisposetwiceIntPtr.Zero(inert)Enddoes not throwDllNotFoundException/EntryPointNotFoundExceptionBeginis not tried againEndthrows inDisposeEndthrows during the closebeforeAssemblyReloadsubscribers still run; the other token is still ended; a laterDisposeends nothingTest names
EditorExecutionActivityTests: T1Hold_ThenDispose_BeginsOneActivityAndEndsThatToken, T2Dispose_CalledTwice_EndsTheActivityOnce, T3Hold_TwiceOverlapping_EndsEachTokenOnceInEitherOrder(2 cases), T4ReleaseAllAndClose_EndsEveryLiveActivity_AndALaterDisposeEndsNothing, T5Hold_AfterReleaseAllAndClose_DoesNotBeginAnotherActivity, T6Hold_WhenThePlatformReturnsNoToken_ReturnsAHandleThatEndsNothing, T13InertProcessActivityApi_Begin_ReturnsNoToken, T15Hold_WhenThePlatformLacksTheNativeEntryPoints_RunsWithoutAnActivity_AndDoesNotTryAgain(2 cases), T16Dispose_WhenEndThrows_DoesNotEndTheSameTokenAgain, T17ReleaseAllAndClose_WhenEndThrows_StillEndsTheOthers_AndEndsNoTokenTwiceUnityCliLoopExecutionRouterActivityTests: T7ExecuteAsync_Tool_HoldsTheActivityWhileTheToolIsPending_AndReleasesItWhenItCompletes, T8ExecuteAsync_ToolThrows_ReleasesTheActivity_AndRethrows, T9ExecuteAsync_InternalCommandCanceledBeforeItStarts_ReleasesTheActivity, T10ExecuteAsync_EditorStatus_DoesNotBeginAnActivity, T11ExecuteAsync_InternalCommand_HoldsAndReleasesAnActivityMacProcessActivityApiTests: T12BeginThenEnd_OnMacOS_ReturnsATokenAndDoesNotThrow, T14End_WithNoToken_ThrowsMutations
Each mutation was applied alone, and every one made at least one test fail.
Releasedoes not callEndReleaseends a token that is not in the setget-editor-statusearly return also holdsReleaseends the token before removing itT7 originally used a tool that completed synchronously, so the whole call finished before
ExecuteAsyncreturned, and all five router tests still passed under m13. It now uses a tool whose task stays pending, and checks the activity is still held while it is pending.Verification
Checked on Unity 2022.3.62f3 and macOS 26.4.1 (Apple silicon).
EditorExecutionActivityTests,UnityCliLoopExecutionRouterActivityTests, andMacProcessActivityApiTestspassed 19/19, with none skipped (the real-OS test ran).endActivity:when a reload closed the registry and a command's hold was disposed afterwards, ending the same token twice. The tests use a fake API, so the switch does not change their results.JsonRpcRequestProcessorTests,JsonRpcRequestProcessorCliVersionGateTests,JsonRpcResponseFactoryWireShapeCharacterizationTests,UnityCliLoopBridgeClientSessionManagerTests,UnityCliLoopBridgeServerLoopTests,UnityCliLoopBridgeServerInstanceFactoryTests, andUnityCliLoopToolRegistryTestspassed 88/88.scripts/check-code-complexity.sh,scripts/check-file-length.sh,scripts/check-dead-code.shwith the CI arguments, and the asmdef policy check passed.compile --force-recompilethree times with 180 s idle after each. Throttling came back 63.2, 63.1, and 63.4 s after the compile returned, so no activity outlived a reload.BeginandEndpair costs about 4 µs inside the Editor (median of 100 pairs; the slowest took 0.11 ms).Not covered
Work the Editor continues after a command has responded is not covered:
compile: the domain reload after the response.control-play-mode: entering or leaving Play Mode and its reload.execute-dynamic-code, when it waits for a domain reload.run-testsin Play Mode: the run continues in the new domain.record-video,replay-input, andenable-watch: per-frame work after they return.set-code-optimization-debug.Holding for a while after the last command was not adopted: accepting a command is delayed by only tens of milliseconds, and a hold across reloads would have to carry state over.
Background throttling on Windows and Linux has not been investigated.
There is no setting or environment variable to turn this off.
get-editor-statusdoes not hold an activity.Speeding up hot reload's patch stage itself is separate work.