FIX: Mitigate memory allocation driven by reflective introspection of binding types ISXB-1766 #2285

ekcoh · 2025-11-18T17:24:58Z

Description

Mitigates effects reported in ISXB-1766 (internal).

The goal is to reduce memory allocations from reflective type loading to only happen when an unresolved type is encountered in an input action asset. The drawback of this is that GC pressure may be postponed from outside intialization. The benefit is that no reflective type loading occurs if no custom type extensions are referenced from .inputaction assets, which would previously unconditionally load an introspect all assemblies unconditionally.

Some scenarios:

In case PWIA or a regular input actions asset do not reference custom processors, interactions or composites, reflection based type registration never runs.
In case PWIA references custom processors, interactions or composites, reflection based type registration is lazily trigger as part of the initialisation sequence of the Input System, and hence there is not much difference to before.
In case regular input actions asset reference custom processors, interactions or composites, the reflection based type registration is triggered when the asset is resolved instead of during initialisation which could be harmful (GC pressure and slow execution). This can in turn be mitigated by doing explicit type registrations before enabling the actions which cause the reflection based type registration to never run.

Testing status & QA

Only quickly verified with a custom processor type in a local project so far.

Overall Product Risks

May have halo effects, e.g. on XRI.

Complexity: small
Halo Effect: large

Comments to reviewers

This doesn't solve the reported behavior, only reduces to number of occasions when it happens.

Checklist

Before review:

Changelog entry added.
- Explains the change in Changed, Fixed, Added sections.
- For API change contains an example snippet and/or migration example.
- JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
- Jira port for the next release set as "Resolved".
Tests added/changed, if applicable.
- Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
- Performance tests.
- Integration tests.
Docs for new/changed API's.
- Xmldoc cross references are set correctly.
- Added explanation how the API works.
- Usage code examples added.
- The manual is updated, if needed.

During merge:

Commit message for squash-merge is prefixed with one of the list:
- NEW: ___.
- FIX: ___.
- DOCS: ___.
- CHANGE: ___.
- RELEASE: 1.1.0-preview.3.

…tive type loading to only happen when an unresolved type is encountered in an input action asset.

u-pr-agent · 2025-11-18T19:03:00Z

PR Reviewer Guide 🔍

(Review updated until commit `f611a2d`)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 ISXB-1766 - Partially compliant Compliant requirements: The PR addresses the memory overhead by changing the reflection-based type scanning from an unconditional startup operation to a lazy-loaded one. The fix prevents memory allocation from reflection in projects that do not use custom types. Non-compliant requirements: [] Requires further human verification: The PR description notes that this change postpones potential GC pressure and slow execution from startup to a later point (e.g., when an `InputActionAsset` with custom types is resolved). This performance trade-off needs to be validated to ensure it doesn't introduce unacceptable stuttering during gameplay. The author states "Only quickly verified with a custom processor type in a local project so far" and mentions a potentially "large" halo effect. Thorough QA testing is required to confirm the fix and check for regressions, especially in related systems like XRI.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪ The code changes are small and implement a straightforward lazy-loading pattern, but understanding the performance trade-offs and potential side effects requires some consideration.
🏅 Score: 88 The PR provides a solid and well-implemented mitigation for the reported memory issue by deferring reflection, but the potential for runtime performance spikes requires further testing and validation.
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Performance Concern The lazy loading of custom types via reflection is now triggered on the first failed type lookup. This could introduce a significant performance spike and GC allocation during gameplay if an `InputActionAsset` with custom types is resolved for the first time, potentially causing a stutter. This trade-off (startup memory vs. runtime performance) should be carefully evaluated. if (m_Manager != null) { if (m_Manager.RegisterCustomTypes()) table.TryGetValue(internedName, out type); }
Update review

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

u-pr-agent · 2025-11-18T19:05:08Z

Suggestion for Packages/com.unity.inputsystem/InputSystem/InputManager.cs (lines 2058-2094):

Suggestion: The current implementation of RegisterCustomTypes is not thread-safe. If multiple threads trigger lazy registration concurrently, they could both pass the hasCustomTypesBeenRegistered check and execute the expensive type scanning logic, causing a race condition. Use a double-checked lock to ensure the registration logic is executed only once. [possible issue, importance: 8]

        // A private static lock object should be added to the class for this.
        private static readonly object s_RegistrationLock = new object();

        internal void RegisterCustomTypes()
        {
            if (hasCustomTypesBeenRegistered)
                return;

            lock (s_RegistrationLock)
            {
                if (hasCustomTypesBeenRegistered)
                    return;

                k_InputRegisterCustomTypesMarker.Begin();

                var inputSystemAssembly = typeof(InputProcessor).Assembly;
                var inputSystemName = inputSystemAssembly.GetName().Name;

                foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies())
                {
                    try
                    {
                        // We are only interested in assemblies that reference the input system
                        foreach (var referencedAssembly in assembly.GetReferencedAssemblies())
                        {
                            if (referencedAssembly.Name == inputSystemName)
                            {
                                RegisterCustomTypes(assembly.GetTypes());
                                break;
                            }
                        }
                    }
                    catch (ReflectionTypeLoadException)
                    {
                        continue;
                    }
                }

                k_InputRegisterCustomTypesMarker.End();

                hasCustomTypesBeenRegistered = true;
            }
        }

🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr-agent

codecov-git.colasdn.top · 2025-11-18T19:32:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

@@             Coverage Diff             @@
##           develop    #2285      +/-   ##
===========================================
+ Coverage    76.81%   76.83%   +0.02%     
===========================================
  Files          476      476              
  Lines        88726    88844     +118     
===========================================
+ Hits         68155    68264     +109     
- Misses       20571    20580       +9

Flag	Coverage Δ
inputsystem_MacOS_2022.3	`5.53% <0.00%> (+0.13%)`	⬆️
inputsystem_MacOS_2022.3_project	`75.40% <100.00%> (+0.71%)`	⬆️
inputsystem_MacOS_6000.0	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_MacOS_6000.0_project	`77.33% <100.00%> (+0.72%)`	⬆️
inputsystem_MacOS_6000.2	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_MacOS_6000.2_project	`77.32% <100.00%> (+0.73%)`	⬆️
inputsystem_MacOS_6000.3	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_MacOS_6000.3_project	`77.32% <100.00%> (+0.72%)`	⬆️
inputsystem_MacOS_6000.4	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_MacOS_6000.4_project	`77.34% <100.00%> (+0.72%)`	⬆️
inputsystem_MacOS_6000.5	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_MacOS_6000.5_project	`77.33% <100.00%> (+0.72%)`	⬆️
inputsystem_Ubuntu_2022.3	`5.53% <0.00%> (+0.13%)`	⬆️
inputsystem_Ubuntu_2022.3_project	`75.20% <100.00%> (+0.71%)`	⬆️
inputsystem_Ubuntu_6000.0	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_Ubuntu_6000.0_project	`77.13% <100.00%> (+0.73%)`	⬆️
inputsystem_Ubuntu_6000.2	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_Ubuntu_6000.2_project	`77.13% <100.00%> (+0.73%)`	⬆️
inputsystem_Ubuntu_6000.3	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_Ubuntu_6000.3_project	`77.13% <100.00%> (+0.72%)`	⬆️
inputsystem_Ubuntu_6000.4	`5.32% <0.00%> (+0.13%)`	⬆️
inputsystem_Ubuntu_6000.4_project	`77.14% <100.00%> (+0.72%)`	⬆️
inputsystem_Ubuntu_6000.5	`5.32% <0.00%> (+0.13%)`	⬆️
inputsystem_Ubuntu_6000.5_project	`77.14% <100.00%> (+0.73%)`	⬆️
inputsystem_Windows_2022.3	`5.53% <0.00%> (+0.13%)`	⬆️
inputsystem_Windows_2022.3_project	`75.53% <100.00%> (+0.71%)`	⬆️
inputsystem_Windows_6000.0	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_Windows_6000.0_project	`77.45% <100.00%> (+0.73%)`	⬆️
inputsystem_Windows_6000.2	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_Windows_6000.2_project	`77.45% <100.00%> (+0.73%)`	⬆️
inputsystem_Windows_6000.3	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_Windows_6000.3_project	`77.45% <100.00%> (+0.73%)`	⬆️
inputsystem_Windows_6000.4	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_Windows_6000.4_project	`77.46% <100.00%> (+0.72%)`	⬆️
inputsystem_Windows_6000.5	`5.31% <0.00%> (+0.13%)`	⬆️
inputsystem_Windows_6000.5_project	`77.46% <100.00%> (+0.73%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../com.unity.inputsystem/InputSystem/InputManager.cs	`89.28% <100.00%> (+0.28%)`	⬆️
...ity.inputsystem/InputSystem/Utilities/TypeTable.cs	`85.71% <100.00%> (+2.73%)`	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

bmalrat · 2025-11-18T19:52:46Z

Packages/com.unity.inputsystem/InputSystem/InputManager.cs

        }

-        void RegisterCustomTypes()
+        internal bool hasCustomTypesBeenRegistered { get; private set; }


should it be also reset after UninstallGlobals or in the init to be sure ?

Overlooked the hidden instance reference to InputManager hiding inside the static API actually using a static push/popped instance. Added a reset in 858f1b7

I agree it's safest to reset since type registration is bound to instate. The assembly type loading result is not.

bmalrat · 2025-11-18T19:55:28Z

Packages/com.unity.inputsystem/InputSystem/Utilities/TypeTable.cs

+            // Failed to look-up type, either type do not exist or it is a custom type.
+            // Check whether we have attempted to load custom types and otherwise lazily load
+            // types only when referenced and reattempt looking up type by name. (ISXB-1766)
+            if (InputSystem.s_Manager == null || InputSystem.s_Manager.hasCustomTypesBeenRegistered)


that's doesn't seems clean to add a global reference to the supposed owner of it.
It breaks the isolation.
We may add to TypeTable a generic fallback call that the owner can configure.

I agree, I added an InputManager reference instead passed via Initialize since it's always "owned" (at least managed) by InputManager. Its strong coupling, but there is currently no point of having an indirection (with a cost).

Addressed in 5cb4617

bmalrat · 2025-11-18T19:58:56Z

Packages/com.unity.inputsystem/InputSystem/Utilities/TypeTable.cs

+                return null;
+
+            InputSystem.s_Manager.RegisterCustomTypes();
+            return LookupTypeRegistration(name);


maybe longer but internedName is already resolved so this should be enough

table.TryGetValue(internedName, out type); return type;

I switched to internedName after also reducing the scope of the recursive call in 5cb4617

…Removed property that could be internal to InputManager and be returned as method result instead.

…ts between InputManager instances.

u-pr-agent · 2025-11-18T22:21:08Z

Persistent review updated to latest commit 7c2cac7

u-pr-agent · 2025-11-19T07:48:54Z

Persistent review updated to latest commit dd3c867

u-pr-agent · 2025-11-19T07:54:21Z

Persistent review updated to latest commit 5f2fe5c

u-pr-agent · 2025-11-19T08:59:02Z

Persistent review updated to latest commit f611a2d

MorganHoarau · 2025-11-19T11:16:40Z

This can in turn be mitigated by doing explicit type registrations before enabling the actions which cause the reflection based type registration to never run.

Is that related to creating input custom processor?
Is this something user can do and if so, is that something documented for users?
- If so, is there documentation update needed to warm about the additional allocation cost if not done properly?

jfreire-unity

I'm approving this so it doesn't get blocked. I think it's the best approach to solve the issue in a short time. It's still unclear what the best solution will be going forward, so perhaps we should track that work after this lands.

My only question is about the steps to validate this. How can I validate (and QA as well) that this change doesn't allocate the +19MB of memory? Would there be a way to automate this and add a test for it?

ekcoh · 2025-11-19T14:24:45Z

This can in turn be mitigated by doing explicit type registrations before enabling the actions which cause the reflection based type registration to never run.

Is that related to creating input custom processor?

Is this something user can do and if so, is that something documented for users?

If so, is there documentation update needed to warm about the additional allocation cost if not done properly?

Yes, it's documented in the manual. It was easier to decipher before Processor page was edited in recent versions IMO, but it is at least in API docs linked from "Remarks" here: https://docs.unity3d.com/Packages/[email protected]/api/UnityEngine.InputSystem.InputSystem.html
Before the "reflective solution" was added, this was the only way to do it which worked fine until Project-wide-input-actions was added since then the registration needs to happen before "SubsystemRegistration" which is not possible and hence the type registration changed to the "reflective solution" which drives the registration from within Input System initialisation instead of requiring the "user" to do it before enabling action assets.

bmalrat

sounds good to me to mitigate the issue

ekcoh · 2025-11-19T14:30:39Z

I'm approving this so it doesn't get blocked. I think it's the best approach to solve the issue in a short time. It's still unclear what the best solution will be going forward, so perhaps we should track that work after this lands.

My only question is about the steps to validate this. How can I validate (and QA as well) that this change doesn't allocate the +19MB of memory? Would there be a way to automate this and add a test for it?

IMO, the potential solution I can see which requires a bit of exploration would be to:

Defer enabling PWIA until just before OnBeforeSceneLoad - might require custom hook since it needs to happen after SubsystemRegistration but before OnBeforeSceneLoad.
Add a source generator that would pick up all related types via implemented interfaces and generate in-memory source that explicitly registers these types using a SubsystemRegistration hook. (Cannot be OnBeforeSceneLoad since it's too late).
Remove reflection type registration code since no longer needed.

See internal JIRA for more details.

bmalrat · 2025-11-19T14:31:04Z

I'm approving this so it doesn't get blocked. I think it's the best approach to solve the issue in a short time. It's still unclear what the best solution will be going forward, so perhaps we should track that work after this lands.

My only question is about the steps to validate this. How can I validate (and QA as well) that this change doesn't allocate the +19MB of memory? Would there be a way to automate this and add a test for it?

You may be able to check it within the profiler marker InputManager.RegisterCustomTypes which should not be called.

FIX: Workaround for ISX-1766 to reduce memory allocations from reflec…

d41445f

…tive type loading to only happen when an unresolved type is encountered in an input action asset.

ekcoh requested review from LeoUnity, bmalrat and jfreire-unity November 18, 2025 17:25

ekcoh changed the title ~~FIX: Workaround for ISX-1766~~ FIX: Mitigation for memory allocation driven by Reflective introspection of binding types ISX-1766 Nov 18, 2025

ekcoh changed the title ~~FIX: Mitigation for memory allocation driven by Reflective introspection of binding types ISX-1766~~ FIX: Mitigation for memory allocation driven by reflective introspection of binding types ISX-1766 Nov 18, 2025

ekcoh marked this pull request as ready for review November 18, 2025 19:01

Moved boolean condition to make sure its set even if exception is thrown

4b229a2

bmalrat reviewed Nov 18, 2025

View reviewed changes

ekcoh added 3 commits November 18, 2025 23:04

Changed TypeTable to be tightly coupled to an InputManager instance. …

5cb4617

…Removed property that could be internal to InputManager and be returned as method result instead.

Reset type registration when uninstalling globals to cause side-effec…

858f1b7

…ts between InputManager instances.

Code cleanup.

7c2cac7

Corrected incorrect if statement.

dd3c867

Simplified code and removed recursive call.

5f2fe5c

ekcoh changed the title ~~FIX: Mitigation for memory allocation driven by reflective introspection of binding types ISX-1766~~ FIX: Mitigation for memory allocation driven by reflective introspection of binding types ISXB-1766 Nov 19, 2025

Updated CHANGELOG.md

f611a2d

ekcoh requested a review from bmalrat November 19, 2025 08:02

jfreire-unity changed the title ~~FIX: Mitigation for memory allocation driven by reflective introspection of binding types ISXB-1766~~ FIX: Mitigate memory allocation driven by reflective introspection of binding types ISXB-1766 Nov 19, 2025

jfreire-unity approved these changes Nov 19, 2025

View reviewed changes

bmalrat approved these changes Nov 19, 2025

View reviewed changes

LeoUnity approved these changes Nov 19, 2025

View reviewed changes

FIX: Mitigate memory allocation driven by reflective introspection of binding types ISXB-1766 #2285

Are you sure you want to change the base?

FIX: Mitigate memory allocation driven by reflective introspection of binding types ISXB-1766 #2285

Conversation

ekcoh commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing status & QA

Overall Product Risks

Comments to reviewers

Checklist

Uh oh!

u-pr-agent bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit f611a2d)

Uh oh!

u-pr-agent bot commented Nov 18, 2025

Uh oh!

codecov-git.colasdn.top bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

u-pr-agent bot commented Nov 18, 2025

Uh oh!

u-pr-agent bot commented Nov 19, 2025

Uh oh!

u-pr-agent bot commented Nov 19, 2025

Uh oh!

u-pr-agent bot commented Nov 19, 2025

Uh oh!

MorganHoarau commented Nov 19, 2025

Uh oh!

jfreire-unity left a comment

Choose a reason for hiding this comment

Uh oh!

ekcoh commented Nov 19, 2025

Uh oh!

bmalrat left a comment

Choose a reason for hiding this comment

Uh oh!

ekcoh commented Nov 19, 2025

Uh oh!

bmalrat commented Nov 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ekcoh commented Nov 18, 2025 •

edited

Loading

u-pr-agent bot commented Nov 18, 2025 •

edited

Loading

(Review updated until commit `f611a2d`)

codecov-git.colasdn.top bot commented Nov 18, 2025 •

edited

Loading