FIX: Bad performance in InputAction.controls when empty (case 1347829, #1437).

Author: Rene Damm

Assets/Tests/InputSystem/CorePerformanceTests.cs

@@ -410,6 +410,62 @@ public void Performance_Rebinding_OneSuccessfulCycle()
             .Run();
     }
 
+    public enum LookupByName
+    {
+        CaseMatches,
+        CaseDoesNotMatch
+    }
+
+    [Test, Performance]
+    [Category("Performance")]
+    [TestCase(LookupByName.CaseMatches)]
+    [TestCase(LookupByName.CaseDoesNotMatch)]
+    public void Performance_LookupActionByName(LookupByName lookup)
+    {
+        const int kActionCount = 100;
+
+        var asset = ScriptableObject.CreateInstance<InputActionAsset>();
+        var map = asset.AddActionMap("map");
+        for (var n = 0; n < kActionCount; ++n)
+            map.AddAction("action" + n);
+
+        Measure.Method(() =>
+        {
+            var _ = asset[(lookup == LookupByName.CaseDoesNotMatch ? "ACTION" : "action") + (int)(kActionCount * 0.75f)];
+        })
+            .MeasurementCount(100)
+            .WarmupCount(5)
+            .Run();
+    }
+
+    [Test, Performance]
+    [Category("Performance")]
+    public void Performance_LookupActionByGuid()
+    {
+        const int kActionCount = 100;
+
+        InputAction actionToFind = null;
+
+        var asset = ScriptableObject.CreateInstance<InputActionAsset>();
+        var map = asset.AddActionMap("map");
+        for (var n = 0; n < kActionCount; ++n)
+        {
+            var action = map.AddAction("action" + n);
+            action.GenerateId();
+
+            if (n == (int)(kActionCount * 0.75f))
+                actionToFind = action;
+        }
+
+        Measure.Method(() =>
+        {
+            Assert.That(asset[actionToFind.id.ToString()], Is.SameAs(actionToFind));
+        })
+            .MeasurementCount(100)
+            .WarmupCount(5)
+            .Run();
+    }
+
     // We're hitting MatchesPrefix a lot from rebinding, so make sure it's performing reasonably well.
     [Test, Performance]
     [Category("Performance")]

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -42,6 +42,8 @@ however, it has to be formatted properly to pass verification tests.
 - Fixed interactions such as `Press` not getting processed correctly when having multiple of them on different bindings of the same action and receiving simultaneous input on all of them ([case 1364667](https://issuetracker.unity3d.com/issues/new-input-system-stops-working-after-pressing-2-keyboard-buttons-at-the-same-time)).
   * If, for example, you bind the A and S key on the same action, put a `Press` interaction on both, and then press both keys, interactions would get missed or got stuck.
 - Fixed `InputAction.IsPressed`/`WasPressed`/`WasReleased` returning incorrect results when binding multiple buttons on the same action and pressing/releasing them simultaneously.
+- Improved performance of looking up actions by name.
+- Fixed `InputAction.controls` exhibiting bad performance when there were no controls bound to an action ([case 1347829](https://issuetracker.unity3d.com/issues/inputaction-dot-controls-are-accessed-slower-when-the-gamepad-slash-controller-is-not-connected)).
 - Fixed interactions involving timeouts (such as `HoldInteraction`) performing erroneous delayed triggers on actions when input is composed of multiple controls ([1251231](https://issuetracker.unity3d.com/issues/input-system-composites-hold-interaction-can-be-performed-when-no-keys-are-hold)).
   * For example, if you bind `Shift+B` using a `OneModifierComposite` and put a `HoldInteraction` on the binding, then depending on the order in which the keys are pressed, you would sometimes see the action spuriously getting triggered when in fact no input was received.
 - Fixed control schemes of bindings not getting updates when being pasted from one `.inputactions` asset into another ([case 1276106](https://issuetracker.unity3d.com/issues/input-system-control-schemes-are-not-resolved-when-copying-bindings-between-inputactionassets)).

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs

@@ -355,6 +355,9 @@ internal int FindActionIndex(string nameOrId)
             if (m_Actions == null)
                 return -1;
 
+            // First time we hit this method, we populate the lookup table.
+            SetUpActionLookupTable();
+
             var actionCount = m_Actions.Length;
 
             var isOldBracedFormat = nameOrId.StartsWith("{") && nameOrId.EndsWith("}");
@@ -368,6 +371,9 @@ internal int FindActionIndex(string nameOrId)
                 }
             }
 
+            if (m_ActionIndexByNameOrId.TryGetValue(nameOrId, out var actionIndex))
+                return actionIndex;
+
             for (var i = 0; i < actionCount; ++i)
             {
                 var action = m_Actions[i];
@@ -378,6 +384,35 @@ internal int FindActionIndex(string nameOrId)
             return InputActionState.kInvalidIndex;
         }
 
+        private void SetUpActionLookupTable()
+        {
+            if (m_ActionIndexByNameOrId != null || m_Actions == null)
+                return;
+
+            m_ActionIndexByNameOrId = new Dictionary<string, int>();
+
+            var actionCount = m_Actions.Length;
+            for (var i = 0; i < actionCount; ++i)
+            {
+                var action = m_Actions[i];
+
+                // We want to make sure an action ID cannot change *after* we have created the table.
+                // NOTE: The *name* of an action, however, *may* change.
+                action.MakeSureIdIsInPlace();
+
+                // We create two lookup paths for each action:
+                // (1) By case-sensitive name.
+                // (2) By GUID string.
+                m_ActionIndexByNameOrId[action.name] = i;
+                m_ActionIndexByNameOrId[action.m_Id] = i;
+            }
+        }
+
+        internal void ClearActionLookupTable()
+        {
+            m_ActionIndexByNameOrId?.Clear();
+        }
+
         private int FindActionIndex(Guid id)
         {
             if (m_Actions == null)
@@ -663,11 +698,14 @@ IEnumerator IEnumerable.GetEnumerator()
         /// isn't, we create a separate array with the bindings sorted by action and have each action reference
         /// a slice through <see cref="InputAction.m_BindingsStartIndex"/> and <see cref="InputAction.m_BindingsCount"/>.
         /// </remarks>
-        /// <seealso cref="SetUpPerActionCachedBindingData"/>
+        /// <seealso cref="SetUpPerActionControlAndBindingArrays"/>
         [NonSerialized] private InputBinding[] m_BindingsForEachAction;
 
         [NonSerialized] private InputControl[] m_ControlsForEachAction;
 
+        [NonSerialized] private bool m_ControlsForEachActionInitialized;
+        [NonSerialized] private bool m_BindingsForEachActionInitialized;
+
         /// <summary>
         /// Number of actions currently enabled in the map.
         /// </summary>
@@ -697,6 +735,8 @@ IEnumerator IEnumerable.GetEnumerator()
 
         [NonSerialized] internal CallbackArray<Action<InputAction.CallbackContext>> m_ActionCallbacks;
 
+        [NonSerialized] internal Dictionary<string, int> m_ActionIndexByNameOrId;
+
         internal static int s_DeferBindingResolution;
 
         internal struct DeviceArray
@@ -780,8 +820,8 @@ internal ReadOnlyArray<InputBinding> GetBindingsForSingleAction(InputAction acti
             Debug.Assert(!action.isSingletonAction || m_SingletonAction == action, "Action is not a singleton action");
 
             // See if we need to refresh.
-            if (m_BindingsForEachAction == null)
-                SetUpPerActionCachedBindingData();
+            if (!m_BindingsForEachActionInitialized)
+                SetUpPerActionControlAndBindingArrays();
 
             return new ReadOnlyArray<InputBinding>(m_BindingsForEachAction, action.m_BindingsStartIndex,
                 action.m_BindingsCount);
@@ -796,8 +836,8 @@ internal ReadOnlyArray<InputControl> GetControlsForSingleAction(InputAction acti
             Debug.Assert(action.m_ActionMap == this);
             Debug.Assert(!action.isSingletonAction || m_SingletonAction == action);
 
-            if (m_ControlsForEachAction == null)
-                SetUpPerActionCachedBindingData();
+            if (!m_ControlsForEachActionInitialized)
+                SetUpPerActionControlAndBindingArrays();
 
             return new ReadOnlyArray<InputControl>(m_ControlsForEachAction, action.m_ControlStartIndex,
                 action.m_ControlCount);
@@ -815,11 +855,17 @@ internal ReadOnlyArray<InputControl> GetControlsForSingleAction(InputAction acti
         /// controls yet (i.e. <see cref="m_State"/> is <c>null</c>). Otherwise, using <see cref="InputAction.bindings"/>
         /// may trigger a control resolution which would be surprising.
         /// </remarks>
-        private unsafe void SetUpPerActionCachedBindingData()
+        private unsafe void SetUpPerActionControlAndBindingArrays()
         {
             // Handle case where we don't have any bindings.
             if (m_Bindings == null)
+            {
+                m_ControlsForEachAction = null;
+                m_BindingsForEachAction = null;
+                m_ControlsForEachActionInitialized = true;
+                m_BindingsForEachActionInitialized = true;
                 return;
+            }
 
             if (m_SingletonAction != null)
             {
@@ -1003,13 +1049,19 @@ private unsafe void SetUpPerActionCachedBindingData()
                     m_BindingsForEachAction = newBindingsArray;
                 }
             }
+
+            m_ControlsForEachActionInitialized = true;
+            m_BindingsForEachActionInitialized = true;
         }
 
         ////TODO: re-use allocations such that only grow the arrays and hit zero GC allocs when we already have enough memory
-        internal void ClearPerActionCachedBindingData()
+        internal void ClearCachedActionData()
         {
-            m_BindingsForEachAction = null;
-            m_ControlsForEachAction = null;
+            m_BindingsForEachActionInitialized = default;
+            m_ControlsForEachActionInitialized = default;
+            m_BindingsForEachAction = default;
+            m_ControlsForEachAction = default;
+            m_ActionIndexByNameOrId = default;
         }
 
         internal void GenerateId()
@@ -1025,6 +1077,7 @@ internal bool LazyResolveBindings()
         {
             // Clear cached controls for actions. Don't need to necessarily clear m_BindingsForEachAction.
             m_ControlsForEachAction = null;
+            m_ControlsForEachActionInitialized = false;
 
             // If we haven't had to resolve bindings yet, we can wait until when we
             // actually have to.
@@ -1085,6 +1138,8 @@ internal void ResolveBindingsIfNecessary()
         /// </remarks>
         internal void ResolveBindings()
         {
+            m_ControlsForEachActionInitialized = false;
+
             // 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())
@@ -1196,6 +1251,7 @@ internal void ResolveBindings()
 
                         ////TODO: determine whether we really need to wipe this; keep them if nothing has changed
                         map.m_ControlsForEachAction = null;
+                        map.m_ControlsForEachActionInitialized = false;
 
                         if (map.m_SingletonAction != null)
                             InputActionState.NotifyListenersOfActionChange(InputActionChange.BoundControlsChanged, map.m_SingletonAction);
@@ -1807,7 +1863,8 @@ public void OnAfterDeserialize()
 
             // Make sure we don't retain any cached per-action data when using serialization
             // to doctor around in action map configurations in the editor.
-            ClearPerActionCachedBindingData();
+            ClearCachedActionData();
+            ClearActionLookupTable();
         }
 
         #endregion

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionRebindingExtensions.cs

@@ -706,7 +706,7 @@ public static int ApplyBindingOverride(this InputActionMap actionMap, InputBindi
 
             if (matchCount > 0)
             {
-                actionMap.ClearPerActionCachedBindingData();
+                actionMap.ClearCachedActionData();
                 actionMap.LazyResolveBindings();
             }
 
@@ -740,7 +740,7 @@ public static void ApplyBindingOverride(this InputActionMap actionMap, int bindi
             actionMap.m_Bindings[bindingIndex].overrideInteractions = bindingOverride.overrideInteractions;
             actionMap.m_Bindings[bindingIndex].overrideProcessors = bindingOverride.overrideProcessors;
 
-            actionMap.ClearPerActionCachedBindingData();
+            actionMap.ClearCachedActionData();
             actionMap.LazyResolveBindings();
         }
 
@@ -835,7 +835,7 @@ public static void RemoveAllBindingOverrides(this IInputActionCollection2 action
                         binding.RemoveOverrides();
                     }
 
-                    actionMap.ClearPerActionCachedBindingData();
+                    actionMap.ClearCachedActionData();
                     actionMap.LazyResolveBindings();
                 }
             }
@@ -874,7 +874,7 @@ public static void RemoveAllBindingOverrides(this InputAction action)
                 bindings[i].overrideProcessors = null;
             }
 
-            actionMap.ClearPerActionCachedBindingData();
+            actionMap.ClearCachedActionData();
             actionMap.LazyResolveBindings();
         }
 

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionSetupExtensions.cs

@@ -201,7 +201,7 @@ public static InputAction AddAction(this InputActionMap map, string name, InputA
             if (map.asset != null)
                 map.asset.MarkAsDirty();
 
-            map.ClearPerActionCachedBindingData();
+            map.ClearCachedActionData();
             map.LazyResolveBindings();
 
             return action;
@@ -246,7 +246,7 @@ public static void RemoveAction(this InputAction action)
             if (actionMap.asset != null)
                 actionMap.asset.MarkAsDirty();
 
-            actionMap.ClearPerActionCachedBindingData();
+            actionMap.ClearCachedActionData();
 
             // Remove bindings to action from map.
             var newActionMapBindingCount = actionMap.m_Bindings.Length - bindingsForAction.Length;
@@ -508,7 +508,7 @@ private static int AddBindingInternal(InputActionMap map, InputBinding binding,
 
             // Invalidate per-action binding sets so that this gets refreshed if
             // anyone queries it.
-            map.ClearPerActionCachedBindingData();
+            map.ClearCachedActionData();
 
             // Make sure bindings get re-resolved.
             map.LazyResolveBindings();
@@ -836,6 +836,7 @@ public static void Rename(this InputAction action, string newName)
 
             var oldName = action.m_Name;
             action.m_Name = newName;
+            actionMap?.ClearActionLookupTable();
 
             if (actionMap?.asset != null)
                 actionMap?.asset.MarkAsDirty();
@@ -1058,7 +1059,7 @@ public BindingSyntax WithName(string name)
                 if (!valid)
                     throw new InvalidOperationException("Accessor is not valid");
                 m_ActionMap.m_Bindings[m_BindingIndexInMap].name = name;
-                m_ActionMap.ClearPerActionCachedBindingData();
+                m_ActionMap.ClearCachedActionData();
                 m_ActionMap.LazyResolveBindings();
                 return this;
             }
@@ -1075,7 +1076,7 @@ public BindingSyntax WithPath(string path)
                 if (!valid)
                     throw new InvalidOperationException("Accessor is not valid");
                 m_ActionMap.m_Bindings[m_BindingIndexInMap].path = path;
-                m_ActionMap.ClearPerActionCachedBindingData();
+                m_ActionMap.ClearCachedActionData();
                 m_ActionMap.LazyResolveBindings();
                 return this;
             }
@@ -1114,7 +1115,7 @@ public BindingSyntax WithGroups(string groups)
 
                 // Set groups on binding.
                 m_ActionMap.m_Bindings[m_BindingIndexInMap].groups = groups;
-                m_ActionMap.ClearPerActionCachedBindingData();
+                m_ActionMap.ClearCachedActionData();
                 m_ActionMap.LazyResolveBindings();
 
                 return this;
@@ -1147,7 +1148,7 @@ public BindingSyntax WithInteractions(string interactions)
 
                 // Set interactions on binding.
                 m_ActionMap.m_Bindings[m_BindingIndexInMap].interactions = interactions;
-                m_ActionMap.ClearPerActionCachedBindingData();
+                m_ActionMap.ClearCachedActionData();
                 m_ActionMap.LazyResolveBindings();
 
                 return this;
@@ -1193,7 +1194,7 @@ public BindingSyntax WithProcessors(string processors)
 
                 // Set processors on binding.
                 m_ActionMap.m_Bindings[m_BindingIndexInMap].processors = processors;
-                m_ActionMap.ClearPerActionCachedBindingData();
+                m_ActionMap.ClearCachedActionData();
                 m_ActionMap.LazyResolveBindings();
 
                 return this;
@@ -1221,7 +1222,7 @@ public BindingSyntax Triggering(InputAction action)
                     throw new ArgumentException(
                         $"Cannot change the action a binding triggers on singleton action '{action}'", nameof(action));
                 m_ActionMap.m_Bindings[m_BindingIndexInMap].action = action.name;
-                m_ActionMap.ClearPerActionCachedBindingData();
+                m_ActionMap.ClearCachedActionData();
                 m_ActionMap.LazyResolveBindings();
                 return this;
             }
@@ -1242,7 +1243,7 @@ public BindingSyntax To(InputBinding binding)
                     throw new InvalidOperationException("Accessor is not valid");
 
                 m_ActionMap.m_Bindings[m_BindingIndexInMap] = binding;
-                m_ActionMap.ClearPerActionCachedBindingData();
+                m_ActionMap.ClearCachedActionData();
                 m_ActionMap.LazyResolveBindings();
 
                 // If it's a singleton action, we force the binding to stay with the action.
@@ -1464,7 +1465,7 @@ public void Erase()
                         ArrayHelpers.EraseAt(ref m_ActionMap.m_Bindings, m_BindingIndexInMap);
                 }
 
-                m_ActionMap.ClearPerActionCachedBindingData();
+                m_ActionMap.ClearCachedActionData();
                 m_ActionMap.LazyResolveBindings();
 
                 // We have switched to a different binding array. For singleton actions, we need to