From 74df3ca716515dd91bf203ab214d0f2ffb290b38 Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Wed, 20 Nov 2024 14:30:30 -0800 Subject: [PATCH 1/9] Add profiler markers for measuring enabling/disabling of InputActions and for binding resolution. --- .../InputSystem/Actions/InputAction.cs | 7 +++++++ .../InputSystem/Actions/InputActionMap.cs | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs index c5fbf9f2ae..689f173e8c 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs @@ -1,5 +1,6 @@ using System; using Unity.Collections.LowLevel.Unsafe; +using Unity.Profiling; using UnityEngine.InputSystem.Controls; using UnityEngine.InputSystem.LowLevel; using UnityEngine.InputSystem.Utilities; @@ -681,6 +682,11 @@ public bool wantsInitialStateCheck } } + /// + /// ProfilerMarker for measuring the enabling/disabling of InputActions. + /// + static readonly ProfilerMarker k_InputActionEnableProfilerMarker = new ProfilerMarker("InputAction.Enable"); + /// /// Construct an unnamed, free-standing action that is not part of any map or asset /// and has no bindings. Bindings can be added with actionTriggered remove => m_ActionCallbacks.RemoveCallback(value); } + /// + /// ProfilerMarker to measure how long it takes to resolve bindings. + /// + static readonly ProfilerMarker k_ResolveBindingsProfilerMarker = new("InputActionMap.ResolveBindings"); + /// /// Construct an action map with default values. /// @@ -1299,6 +1305,7 @@ internal bool ResolveBindingsIfNecessary() /// internal void ResolveBindings() { + k_ResolveBindingsProfilerMarker.Auto(); // Make sure that if we trigger callbacks as part of disabling and re-enabling actions, // we don't trigger a re-resolve while we're already resolving bindings. using (InputActionRebindingExtensions.DeferBindingResolution()) From fb158e1d654ea7bff1d3db31be9e4a5d864cba88 Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Wed, 20 Nov 2024 15:05:30 -0800 Subject: [PATCH 2/9] Add changelog entry, fix compile error for 2019.4. --- Packages/com.unity.inputsystem/CHANGELOG.md | 1 + .../com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 21c891a88c..aab8c0ebe8 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -42,6 +42,7 @@ however, it has to be formatted properly to pass verification tests. ### Added - Added new API `InputSystem.settings.useIMGUIEditorForAssets` that should be used in custom `InputParameterEditor` that use both IMGUI and UI Toolkit. +- Added ProfilerMakers to `InputAction.Enable()` and `InputActionMap.ResolveBindings()` to enable gathering of profiling data. ## [1.11.2] - 2024-10-16 diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs index 9da504eeec..2f556e4f25 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs @@ -317,7 +317,7 @@ public event Action actionTriggered /// /// ProfilerMarker to measure how long it takes to resolve bindings. /// - static readonly ProfilerMarker k_ResolveBindingsProfilerMarker = new("InputActionMap.ResolveBindings"); + static readonly ProfilerMarker k_ResolveBindingsProfilerMarker = new ProfilerMarker("InputActionMap.ResolveBindings"); /// /// Construct an action map with default values. From cef6c1b54576d3b45cf48ee2b5c0ff861456aef7 Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Thu, 21 Nov 2024 11:20:25 -0800 Subject: [PATCH 3/9] Add ProfilerMarkers to CorePerformanceTests. --- Assets/Tests/InputSystem/CorePerformanceTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/InputSystem/CorePerformanceTests.cs b/Assets/Tests/InputSystem/CorePerformanceTests.cs index 5d7d8ac230..cbd26eff41 100644 --- a/Assets/Tests/InputSystem/CorePerformanceTests.cs +++ b/Assets/Tests/InputSystem/CorePerformanceTests.cs @@ -1113,7 +1113,9 @@ public void Performance_OptimizedControls_ReadingPose4kTimes(OptimizationTestTyp "InputSystem.onAfterUpdate", "PreUpdate.NewInputUpdate", "PreUpdate.InputForUIUpdate", - "FixedUpdate.NewInputFixedUpdate" + "FixedUpdate.NewInputFixedUpdate", + "InputAction.Enable", + "InputActionMap.ResolveBindings" }; [PrebuildSetup(typeof(ProjectWideActionsBuildSetup))] From 8607ba70cfc92fe3197d65dccc6153f052f5c8b2 Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Mon, 25 Nov 2024 09:45:48 -0800 Subject: [PATCH 4/9] Fix profiler maker scope. --- .../InputSystem/Actions/InputAction.cs | 32 ++-- .../InputSystem/Actions/InputActionMap.cs | 159 +++++++++--------- 2 files changed, 100 insertions(+), 91 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs index 689f173e8c..d4c1dd41d5 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputAction.cs @@ -686,6 +686,7 @@ public bool wantsInitialStateCheck /// ProfilerMarker for measuring the enabling/disabling of InputActions. /// static readonly ProfilerMarker k_InputActionEnableProfilerMarker = new ProfilerMarker("InputAction.Enable"); + static readonly ProfilerMarker k_InputActionDisableProfilerMarker = new ProfilerMarker("InputAction.Disable"); /// /// Construct an unnamed, free-standing action that is not part of any map or asset @@ -905,19 +906,21 @@ public override string ToString() /// public void Enable() { - if (enabled) - return; + using (k_InputActionEnableProfilerMarker.Auto()) + { + if (enabled) + return; - k_InputActionEnableProfilerMarker.Auto(); - // For singleton actions, we create an internal-only InputActionMap - // private to the action. - var map = GetOrCreateActionMap(); + // For singleton actions, we create an internal-only InputActionMap + // private to the action. + var map = GetOrCreateActionMap(); - // First time we're enabled, find all controls. - map.ResolveBindingsIfNecessary(); + // First time we're enabled, find all controls. + map.ResolveBindingsIfNecessary(); - // Go live. - map.m_State.EnableSingleAction(this); + // Go live. + map.m_State.EnableSingleAction(this); + } } /// @@ -935,10 +938,13 @@ public void Enable() /// public void Disable() { - if (!enabled) - return; + using (k_InputActionDisableProfilerMarker.Auto()) + { + if (!enabled) + return; - m_ActionMap.m_State.DisableSingleAction(this); + m_ActionMap.m_State.DisableSingleAction(this); + } } ////REVIEW: is *not* cloning IDs here really the right thing to do? diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs index 2f556e4f25..e5b5f2cb2f 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs @@ -1305,101 +1305,104 @@ internal bool ResolveBindingsIfNecessary() /// internal void ResolveBindings() { - k_ResolveBindingsProfilerMarker.Auto(); - // Make sure that if we trigger callbacks as part of disabling and re-enabling actions, - // we don't trigger a re-resolve while we're already resolving bindings. - using (InputActionRebindingExtensions.DeferBindingResolution()) + using (k_ResolveBindingsProfilerMarker.Auto()) { - // In case we have actions that are currently enabled, we temporarily retain the - // UnmanagedMemory of our InputActionState so that we can sync action states after - // we have re-resolved bindings. - var oldMemory = new InputActionState.UnmanagedMemory(); - try + // Make sure that if we trigger callbacks as part of disabling and re-enabling actions, + // we don't trigger a re-resolve while we're already resolving bindings. + using (InputActionRebindingExtensions.DeferBindingResolution()) { - OneOrMore> actionMaps; + // In case we have actions that are currently enabled, we temporarily retain the + // UnmanagedMemory of our InputActionState so that we can sync action states after + // we have re-resolved bindings. + var oldMemory = new InputActionState.UnmanagedMemory(); + try + { + OneOrMore> actionMaps; - // Start resolving. - var resolver = new InputBindingResolver(); + // Start resolving. + var resolver = new InputBindingResolver(); - // If we're part of an asset, we share state and thus binding resolution with - // all maps in the asset. - var needFullResolve = m_State == null; - if (m_Asset != null) - { - actionMaps = m_Asset.actionMaps; - Debug.Assert(actionMaps.Count > 0, "Asset referred to by action map does not have action maps"); + // If we're part of an asset, we share state and thus binding resolution with + // all maps in the asset. + var needFullResolve = m_State == null; + if (m_Asset != null) + { + actionMaps = m_Asset.actionMaps; + Debug.Assert(actionMaps.Count > 0, "Asset referred to by action map does not have action maps"); - // If there's a binding mask set on the asset, apply it. - resolver.bindingMask = m_Asset.m_BindingMask; + // If there's a binding mask set on the asset, apply it. + resolver.bindingMask = m_Asset.m_BindingMask; - foreach (var map in actionMaps) + foreach (var map in actionMaps) + { + needFullResolve |= map.bindingResolutionNeedsFullReResolve; + map.needToResolveBindings = false; + map.bindingResolutionNeedsFullReResolve = false; + map.controlsForEachActionInitialized = false; + } + } + else { - needFullResolve |= map.bindingResolutionNeedsFullReResolve; - map.needToResolveBindings = false; - map.bindingResolutionNeedsFullReResolve = false; - map.controlsForEachActionInitialized = false; + // Standalone action map (possibly a hidden one created for a singleton action). + // Gets its own private state. + + actionMaps = this; + needFullResolve |= bindingResolutionNeedsFullReResolve; + needToResolveBindings = false; + bindingResolutionNeedsFullReResolve = false; + controlsForEachActionInitialized = false; } - } - else - { - // Standalone action map (possibly a hidden one created for a singleton action). - // Gets its own private state. - - actionMaps = this; - needFullResolve |= bindingResolutionNeedsFullReResolve; - needToResolveBindings = false; - bindingResolutionNeedsFullReResolve = false; - controlsForEachActionInitialized = false; - } - // If we already have a state, re-use the arrays we have already allocated. - // NOTE: We will install the arrays on the very same InputActionState instance below. In the - // case where we didn't have to grow the arrays, we should end up with zero GC allocations - // here. - var hasEnabledActions = false; - InputControlList activeControls = default; - if (m_State != null) - { - // Grab a clone of the current memory. We clone because disabling all the actions - // in the map will alter the memory state and we want the state before we start - // touching it. - oldMemory = m_State.memory.Clone(); + // If we already have a state, re-use the arrays we have already allocated. + // NOTE: We will install the arrays on the very same InputActionState instance below. In the + // case where we didn't have to grow the arrays, we should end up with zero GC allocations + // here. + var hasEnabledActions = false; + InputControlList activeControls = default; + if (m_State != null) + { + // Grab a clone of the current memory. We clone because disabling all the actions + // in the map will alter the memory state and we want the state before we start + // touching it. + oldMemory = m_State.memory.Clone(); - m_State.PrepareForBindingReResolution(needFullResolve, ref activeControls, ref hasEnabledActions); + m_State.PrepareForBindingReResolution(needFullResolve, ref activeControls, ref hasEnabledActions); - // Reuse the arrays we have so that we can avoid managed memory allocations, if possible. - resolver.StartWithPreviousResolve(m_State, isFullResolve: needFullResolve); + // Reuse the arrays we have so that we can avoid managed memory allocations, if possible. + resolver.StartWithPreviousResolve(m_State, isFullResolve: needFullResolve); - // Throw away old memory. - m_State.memory.Dispose(); - } + // Throw away old memory. + m_State.memory.Dispose(); + } + + // Resolve all maps in the asset. + foreach (var map in actionMaps) + resolver.AddActionMap(map); - // Resolve all maps in the asset. - foreach (var map in actionMaps) - resolver.AddActionMap(map); + // Install state. + if (m_State == null) + { + m_State = new InputActionState(); + m_State.Initialize(resolver); + } + else + { + m_State.ClaimDataFrom(resolver); + } - // Install state. - if (m_State == null) - { - m_State = new InputActionState(); - m_State.Initialize(resolver); - } - else - { - m_State.ClaimDataFrom(resolver); + if (m_Asset != null) + { + foreach (var map in actionMaps) + map.m_State = m_State; + m_Asset.m_SharedStateForAllMaps = m_State; + } + + m_State.FinishBindingResolution(hasEnabledActions, oldMemory, activeControls, isFullResolve: needFullResolve); } - if (m_Asset != null) + finally { - foreach (var map in actionMaps) - map.m_State = m_State; - m_Asset.m_SharedStateForAllMaps = m_State; + oldMemory.Dispose(); } - - m_State.FinishBindingResolution(hasEnabledActions, oldMemory, activeControls, isFullResolve: needFullResolve); - } - finally - { - oldMemory.Dispose(); } } } From 75d8da19592fcf04d784f7e5ac73a3d6fbf00980 Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Mon, 25 Nov 2024 10:01:33 -0800 Subject: [PATCH 5/9] Add all ProfilerMarkers that exist in the package code. --- .../Tests/InputSystem/CorePerformanceTests.cs | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/Assets/Tests/InputSystem/CorePerformanceTests.cs b/Assets/Tests/InputSystem/CorePerformanceTests.cs index cbd26eff41..2b2fbebe06 100644 --- a/Assets/Tests/InputSystem/CorePerformanceTests.cs +++ b/Assets/Tests/InputSystem/CorePerformanceTests.cs @@ -1108,14 +1108,39 @@ public void Performance_OptimizedControls_ReadingPose4kTimes(OptimizationTestTyp // Needed for the tests below. string[] allInputSystemProfilerMarkers = { + "BuildControlTree", + "FixedUpdate.NewInputFixedUpdate", + "HIDParseDescriptorFallback", + "InitialActionStateCheck", + "InputActionCallback", + "InputActionResolveConflict", + "InputAction.Enable", + "InputAction.Disable", + "InputActionMap.ResolveBindings", + "InputCheckForUnpairedDeviceActivity", + "InputEventTrace", + "InputEventTreeView.BuildRoot", + "InputManager.RegisterCustomTypes", + "InputManager.RestoreDevicesAfterDomainReload", "InputUpdate", - "InputSystem.onBeforeUpdate", + "InputSystem.AddDevice", + "InputSystem.InitializeInEditor", + "InputSystem.onActionsChange", + "InputSystem.onActionChange", "InputSystem.onAfterUpdate", - "PreUpdate.NewInputUpdate", + "InputSystem.onBeforeUpdate", + "InputSystem.onDeviceSettingsChange", + "InputSystem.onEvent", + "InputSystem.onLayoutChange", + "InputSystem.onSettingsChange", + "InputSystem.onDeviceChange", + "InputSystem.Reset", + "InputSystem.TryFindMatchingControlLayout", + "InputUser.onChange", "PreUpdate.InputForUIUpdate", - "FixedUpdate.NewInputFixedUpdate", - "InputAction.Enable", - "InputActionMap.ResolveBindings" + "PreUpdate.NewInputUpdate", + "TouchAllocate", + "Touchscreen.OnNextUpdate" }; [PrebuildSetup(typeof(ProjectWideActionsBuildSetup))] From da9b7ba967a2649f88613290a42bae1cf2e33264 Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Mon, 25 Nov 2024 12:16:21 -0800 Subject: [PATCH 6/9] See if profiler marker is causing test to fail. --- .../com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs index e5b5f2cb2f..645606b115 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs @@ -1305,7 +1305,7 @@ internal bool ResolveBindingsIfNecessary() /// internal void ResolveBindings() { - using (k_ResolveBindingsProfilerMarker.Auto()) + // using (k_ResolveBindingsProfilerMarker.Auto()) { // Make sure that if we trigger callbacks as part of disabling and re-enabling actions, // we don't trigger a re-resolve while we're already resolving bindings. From 2b9baded64629c9e4bffef1f4ed6c28b56a916a9 Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Wed, 4 Dec 2024 13:22:37 -0800 Subject: [PATCH 7/9] Revert "Add all ProfilerMarkers that exist in the package code." This reverts commit 2802da51d3bbb2c244f5de65b6909d09cb4cc791. --- .../Tests/InputSystem/CorePerformanceTests.cs | 35 +++---------------- 1 file changed, 5 insertions(+), 30 deletions(-) diff --git a/Assets/Tests/InputSystem/CorePerformanceTests.cs b/Assets/Tests/InputSystem/CorePerformanceTests.cs index 2b2fbebe06..cbd26eff41 100644 --- a/Assets/Tests/InputSystem/CorePerformanceTests.cs +++ b/Assets/Tests/InputSystem/CorePerformanceTests.cs @@ -1108,39 +1108,14 @@ public void Performance_OptimizedControls_ReadingPose4kTimes(OptimizationTestTyp // Needed for the tests below. string[] allInputSystemProfilerMarkers = { - "BuildControlTree", - "FixedUpdate.NewInputFixedUpdate", - "HIDParseDescriptorFallback", - "InitialActionStateCheck", - "InputActionCallback", - "InputActionResolveConflict", - "InputAction.Enable", - "InputAction.Disable", - "InputActionMap.ResolveBindings", - "InputCheckForUnpairedDeviceActivity", - "InputEventTrace", - "InputEventTreeView.BuildRoot", - "InputManager.RegisterCustomTypes", - "InputManager.RestoreDevicesAfterDomainReload", "InputUpdate", - "InputSystem.AddDevice", - "InputSystem.InitializeInEditor", - "InputSystem.onActionsChange", - "InputSystem.onActionChange", - "InputSystem.onAfterUpdate", "InputSystem.onBeforeUpdate", - "InputSystem.onDeviceSettingsChange", - "InputSystem.onEvent", - "InputSystem.onLayoutChange", - "InputSystem.onSettingsChange", - "InputSystem.onDeviceChange", - "InputSystem.Reset", - "InputSystem.TryFindMatchingControlLayout", - "InputUser.onChange", - "PreUpdate.InputForUIUpdate", + "InputSystem.onAfterUpdate", "PreUpdate.NewInputUpdate", - "TouchAllocate", - "Touchscreen.OnNextUpdate" + "PreUpdate.InputForUIUpdate", + "FixedUpdate.NewInputFixedUpdate", + "InputAction.Enable", + "InputActionMap.ResolveBindings" }; [PrebuildSetup(typeof(ProjectWideActionsBuildSetup))] From 7cc322da62d9ae9bc8efab9f3779d60d8b71454e Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Wed, 4 Dec 2024 13:26:23 -0800 Subject: [PATCH 8/9] Fix compile error in test and uncomment profiler marker. --- Assets/Tests/InputSystem/Plugins/UITests.cs | 2 ++ .../com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Assets/Tests/InputSystem/Plugins/UITests.cs b/Assets/Tests/InputSystem/Plugins/UITests.cs index f098df98a6..1b94f75ab8 100644 --- a/Assets/Tests/InputSystem/Plugins/UITests.cs +++ b/Assets/Tests/InputSystem/Plugins/UITests.cs @@ -1793,6 +1793,7 @@ public IEnumerator UI_CanReleaseAndPressTouchesOnSameFrame() .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition)); +#if UNITY_2021_2_OR_NEWER Assert.That(scene.rightChildReceiver.events, Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerMove).And .Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And @@ -1800,6 +1801,7 @@ public IEnumerator UI_CanReleaseAndPressTouchesOnSameFrame() .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And .Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And .Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition)); +#endif // Pointer 3 Assert.That(scene.rightChildReceiver.events, diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs index 645606b115..e5b5f2cb2f 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs @@ -1305,7 +1305,7 @@ internal bool ResolveBindingsIfNecessary() /// internal void ResolveBindings() { - // using (k_ResolveBindingsProfilerMarker.Auto()) + using (k_ResolveBindingsProfilerMarker.Auto()) { // Make sure that if we trigger callbacks as part of disabling and re-enabling actions, // we don't trigger a re-resolve while we're already resolving bindings. From 2e38b4125af02685b2e3adb7f91fc8376f1e64c4 Mon Sep 17 00:00:00 2001 From: Christopher Goy Date: Thu, 5 Dec 2024 13:11:44 -0800 Subject: [PATCH 9/9] Add Disable ProfilerMarker. --- Assets/Tests/InputSystem/CorePerformanceTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Assets/Tests/InputSystem/CorePerformanceTests.cs b/Assets/Tests/InputSystem/CorePerformanceTests.cs index cbd26eff41..86026ed313 100644 --- a/Assets/Tests/InputSystem/CorePerformanceTests.cs +++ b/Assets/Tests/InputSystem/CorePerformanceTests.cs @@ -1114,6 +1114,7 @@ public void Performance_OptimizedControls_ReadingPose4kTimes(OptimizationTestTyp "PreUpdate.NewInputUpdate", "PreUpdate.InputForUIUpdate", "FixedUpdate.NewInputFixedUpdate", + "InputAction.Disable", "InputAction.Enable", "InputActionMap.ResolveBindings" };