Skip to content

CHANGE: Make sure all callbacks are instrumented with ProfilerMarkers instead of Profiler.Begin/EndSample. #1992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 5, 2024
+37 −24
Merged

CHANGE: Make sure all callbacks are instrumented with ProfilerMarkers instead of Profiler.Begin/EndSample. #1992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ad6fb1
Make sure all callbacks are instrumented with ProfilerMarkers instead…
surfnerd Aug 29, 2024
8881803
Fix spacing and string consistency.
surfnerd Aug 29, 2024
f3dd7b1
Match modifiers of local code.
surfnerd Sep 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ internal unsafe class InputActionState : IInputStateChangeMonitor, ICloneable, I
private static readonly ProfilerMarker k_InputInitialActionStateCheckMarker = new ProfilerMarker("InitialActionStateCheck");
private static readonly ProfilerMarker k_InputActionResolveConflictMarker = new ProfilerMarker("InputActionResolveConflict");
private static readonly ProfilerMarker k_InputActionCallbackMarker = new ProfilerMarker("InputActionCallback");
private static readonly ProfilerMarker k_InputOnActionChangeMarker = new ProfilerMarker("InpustSystem.onActionChange");
private static readonly ProfilerMarker k_InputOnDeviceChangeMarker = new ProfilerMarker("InpustSystem.onDeviceChange");

/// <summary>
/// Initialize execution state with given resolved binding information.
Expand Down Expand Up @@ -2548,7 +2550,7 @@ private void CallActionListeners(int actionIndex, InputActionMap actionMap, Inpu
return;
}

DelegateHelpers.InvokeCallbacksSafe(ref s_GlobalState.onActionChange, action, change, "InputSystem.onActionChange");
DelegateHelpers.InvokeCallbacksSafe(ref s_GlobalState.onActionChange, action, change, k_InputOnActionChangeMarker, "InputSystem.onActionChange");
}

// Run callbacks (if any) directly on action.
Expand Down Expand Up @@ -4336,7 +4338,7 @@ internal static void NotifyListenersOfActionChange(InputActionChange change, obj
Debug.Assert(actionOrMapOrAsset is InputAction || (actionOrMapOrAsset as InputActionMap)?.m_SingletonAction == null,
"Must not send notifications for changes made to hidden action maps of singleton actions");

DelegateHelpers.InvokeCallbacksSafe(ref s_GlobalState.onActionChange, actionOrMapOrAsset, change, "onActionChange");
DelegateHelpers.InvokeCallbacksSafe(ref s_GlobalState.onActionChange, actionOrMapOrAsset, change, k_InputOnActionChangeMarker, "InputSystem.onActionChange");
if (change == InputActionChange.BoundControlsChanged)
DelegateHelpers.InvokeCallbacksSafe(ref s_GlobalState.onActionControlsChanged, actionOrMapOrAsset, "onActionControlsChange");
}
Expand Down
42 changes: 26 additions & 16 deletions Packages/com.unity.inputsystem/InputSystem/InputManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,16 @@ internal partial class InputManager
static readonly ProfilerMarker k_InputRestoreDevicesAfterReloadMarker = new ProfilerMarker("InputManager.RestoreDevicesAfterDomainReload");
static readonly ProfilerMarker k_InputRegisterCustomTypesMarker = new ProfilerMarker("InputManager.RegisterCustomTypes");

static readonly ProfilerMarker k_InputOnBeforeUpdateMarker = new ProfilerMarker("InputSystem.onBeforeUpdate");
static readonly ProfilerMarker k_InputOnAfterUpdateMarker = new ProfilerMarker("InputSystem.onAfterUpdate");
static readonly ProfilerMarker k_InputOnSettingsChangeMarker = new ProfilerMarker("InputSystem.onSettingsChange");
static readonly ProfilerMarker k_InputOnDeviceSettingsChangeMarker = new ProfilerMarker("InputSystem.onDeviceSettingsChange");
static readonly ProfilerMarker k_InputOnEventMarker = new ProfilerMarker("InputSystem.onEvent");
static readonly ProfilerMarker k_InputOnLayoutChangeMarker = new ProfilerMarker("InputSystem.onLayoutChange");
static readonly ProfilerMarker k_InputOnDeviceChangeMarker = new ProfilerMarker("InpustSystem.onDeviceChange");
static readonly ProfilerMarker k_InputOnActionsChangeMarker = new ProfilerMarker("InpustSystem.onActionsChange");


public InputMetrics metrics
{
get
Expand Down Expand Up @@ -600,7 +610,7 @@ private void PerformLayoutPostRegistration(InternedString layoutName, InlinedArr

// Let listeners know.
var change = isReplacement ? InputControlLayoutChange.Replaced : InputControlLayoutChange.Added;
DelegateHelpers.InvokeCallbacksSafe(ref m_LayoutChangeListeners, layoutName.ToString(), change, "InputSystem.onLayoutChange");
DelegateHelpers.InvokeCallbacksSafe(ref m_LayoutChangeListeners, layoutName.ToString(), change, k_InputOnLayoutChangeMarker, "InputSystem.onLayoutChange");
}

public void RegisterPrecompiledLayout<TDevice>(string metadata)
Expand Down Expand Up @@ -868,7 +878,7 @@ public void RemoveControlLayout(string name)
//// remove those layouts, too

// Let listeners know.
DelegateHelpers.InvokeCallbacksSafe(ref m_LayoutChangeListeners, name, InputControlLayoutChange.Removed, "InputSystem.onLayoutChange");
DelegateHelpers.InvokeCallbacksSafe(ref m_LayoutChangeListeners, name, InputControlLayoutChange.Removed, k_InputOnLayoutChangeMarker, "InputSystem.onLayoutChange");
}

public InputControlLayout TryLoadControlLayout(Type type)
Expand Down Expand Up @@ -1121,7 +1131,7 @@ private void NotifyUsageChanged(InputDevice device)
InputActionState.OnDeviceChange(device, InputDeviceChange.UsageChanged);

// Notify listeners.
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, InputDeviceChange.UsageChanged, "InputSystem.onDeviceChange");
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, InputDeviceChange.UsageChanged, k_InputOnDeviceChangeMarker, "InputSystem.onDeviceChange");

////REVIEW: This was for the XRController leftHand and rightHand getters but these do lookups dynamically now; remove?
// Usage may affect current device so update.
Expand Down Expand Up @@ -1287,7 +1297,7 @@ public void AddDevice(InputDevice device)
device.MakeCurrent();

// Notify listeners.
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, InputDeviceChange.Added, "InputSystem.onDeviceChange");
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, InputDeviceChange.Added, k_InputOnDeviceChangeMarker, "InputSystem.onDeviceChange");

// Request device to send us an initial state update.
if (device.enabled)
Expand Down Expand Up @@ -1442,7 +1452,7 @@ public void RemoveDevice(InputDevice device, bool keepOnListOfAvailableDevices =
device.NotifyRemoved();

// Let listeners know.
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, InputDeviceChange.Removed, "InputSystem.onDeviceChange");
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, InputDeviceChange.Removed, k_InputOnDeviceChangeMarker, "InputSystem.onDeviceChange");

// Try setting next device of same type as current
InputSystem.GetDevice(device.GetType())?.MakeCurrent();
Expand All @@ -1466,7 +1476,7 @@ public unsafe void ResetDevice(InputDevice device, bool alsoResetDontResetContro
// Trigger reset notification.
var change = isHardReset ? InputDeviceChange.HardReset : InputDeviceChange.SoftReset;
InputActionState.OnDeviceChange(device, change);
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, change, "onDeviceChange");
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, change, k_InputOnDeviceChangeMarker, "InputSystem.onDeviceChange");

// If the device implements its own reset, let it handle it.
if (!alsoResetDontResetControls && device is ICustomDeviceReset customReset)
Expand Down Expand Up @@ -1754,7 +1764,7 @@ public void EnableOrDisableDevice(InputDevice device, bool enable, DeviceDisable

// Let listeners know.
var deviceChange = enable ? InputDeviceChange.Enabled : InputDeviceChange.Disabled;
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, deviceChange, "InputSystem.onDeviceChange");
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, deviceChange, k_InputOnDeviceChangeMarker, "InputSystem.onDeviceChange");
}

private unsafe void QueueEvent(InputEvent* eventPtr)
Expand Down Expand Up @@ -2500,7 +2510,7 @@ private void OnNativeDeviceDiscovered(int deviceId, string deviceDescriptor)
AddDevice(device);

DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners, device, InputDeviceChange.Reconnected,
"InputSystem.onDeviceChange");
k_InputOnDeviceChangeMarker, "InputSystem.onDeviceChange");
}
else
{
Expand Down Expand Up @@ -2709,7 +2719,7 @@ private void OnBeforeUpdate(InputUpdateType updateType)
}
}

DelegateHelpers.InvokeCallbacksSafe(ref m_BeforeUpdateListeners, "onBeforeUpdate");
DelegateHelpers.InvokeCallbacksSafe(ref m_BeforeUpdateListeners, k_InputOnBeforeUpdateMarker, "InputSystem.onBeforeUpdate");
}

/// <summary>
Expand Down Expand Up @@ -2824,14 +2834,14 @@ internal void ApplySettings()

// Let listeners know.
DelegateHelpers.InvokeCallbacksSafe(ref m_SettingsChangedListeners,
"InputSystem.onSettingsChange");
k_InputOnSettingsChangeMarker, "InputSystem.onSettingsChange");
}

#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
internal void ApplyActions()
{
// Let listeners know.
DelegateHelpers.InvokeCallbacksSafe(ref m_ActionsChangedListeners, "InputSystem.onActionsChange");
DelegateHelpers.InvokeCallbacksSafe(ref m_ActionsChangedListeners, k_InputOnActionsChangeMarker, "InputSystem.onActionsChange");
}

#endif
Expand Down Expand Up @@ -3448,7 +3458,7 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
if (m_EventListeners.length > 0)
{
DelegateHelpers.InvokeCallbacksSafe(ref m_EventListeners,
new InputEventPtr(currentEventReadPtr), device, "InputSystem.onEvent");
new InputEventPtr(currentEventReadPtr), device, k_InputOnEventMarker, "InputSystem.onEvent");

// If a listener marks the event as handled, we don't process it further.
if (currentEventReadPtr->handled)
Expand Down Expand Up @@ -3590,7 +3600,7 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
ArrayHelpers.AppendWithCapacity(ref m_DisconnectedDevices,
ref m_DisconnectedDevicesCount, device);
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners,
device, InputDeviceChange.Disconnected, "InputSystem.onDeviceChange");
device, InputDeviceChange.Disconnected, k_InputOnDeviceChangeMarker, "InputSystem.onDeviceChange");
}

break;
Expand All @@ -3600,7 +3610,7 @@ private unsafe void OnUpdate(InputUpdateType updateType, ref InputEventBuffer ev
device.NotifyConfigurationChanged();
InputActionState.OnDeviceChange(device, InputDeviceChange.ConfigurationChanged);
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceChangeListeners,
device, InputDeviceChange.ConfigurationChanged, "InputSystem.onDeviceChange");
device, InputDeviceChange.ConfigurationChanged, k_InputOnDeviceChangeMarker, "InputSystem.onDeviceChange");
break;

case DeviceResetEvent.Type:
Expand Down Expand Up @@ -3716,7 +3726,7 @@ private void InvokeAfterUpdateCallback(InputUpdateType updateType)
return;

DelegateHelpers.InvokeCallbacksSafe(ref m_AfterUpdateListeners,
"InputSystem.onAfterUpdate");
k_InputOnAfterUpdateMarker, "InputSystem.onAfterUpdate");
}

private bool m_ShouldMakeCurrentlyUpdatingDeviceCurrent;
Expand Down Expand Up @@ -3897,7 +3907,7 @@ internal unsafe bool UpdateState(InputDevice device, InputUpdateType updateType,

// Notify listeners.
DelegateHelpers.InvokeCallbacksSafe(ref m_DeviceStateChangeListeners,
device, eventPtr, "InputSystem.onDeviceStateChange");
device, eventPtr, k_InputOnDeviceSettingsChangeMarker, "InputSystem.onDeviceStateChange");

// Now that we've committed the new state to memory, if any of the change
// monitors fired, let the associated actions know.
Expand Down
13 changes: 7 additions & 6 deletions Packages/com.unity.inputsystem/InputSystem/Utilities/DelegateHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using Unity.Profiling;

namespace UnityEngine.InputSystem.Utilities
{
Expand All @@ -7,11 +8,11 @@ internal static class DelegateHelpers
// InvokeCallbacksSafe protects both against the callback getting removed while being called
// and against exceptions being thrown by the callback.

public static void InvokeCallbacksSafe(ref CallbackArray<Action> callbacks, string callbackName, object context = null)
public static void InvokeCallbacksSafe(ref CallbackArray<Action> callbacks, ProfilerMarker marker, string callbackName, object context = null)
{
if (callbacks.length == 0)
return;
Profiling.Profiler.BeginSample(callbackName);
marker.Begin();
callbacks.LockForChanges();
for (var i = 0; i < callbacks.length; ++i)
{
Expand All @@ -29,7 +30,7 @@ public static void InvokeCallbacksSafe(ref CallbackArray<Action> callbacks, stri
}
}
callbacks.UnlockForChanges();
Profiling.Profiler.EndSample();
marker.End();
}

public static void InvokeCallbacksSafe<TValue>(ref CallbackArray<Action<TValue>> callbacks, TValue argument, string callbackName, object context = null)
Expand Down Expand Up @@ -57,11 +58,11 @@ public static void InvokeCallbacksSafe<TValue>(ref CallbackArray<Action<TValue>>
Profiling.Profiler.EndSample();
}

public static void InvokeCallbacksSafe<TValue1, TValue2>(ref CallbackArray<Action<TValue1, TValue2>> callbacks, TValue1 argument1, TValue2 argument2, string callbackName, object context = null)
public static void InvokeCallbacksSafe<TValue1, TValue2>(ref CallbackArray<Action<TValue1, TValue2>> callbacks, TValue1 argument1, TValue2 argument2, ProfilerMarker marker, string callbackName, object context = null)
{
if (callbacks.length == 0)
return;
Profiling.Profiler.BeginSample(callbackName);
marker.Begin();
callbacks.LockForChanges();
for (var i = 0; i < callbacks.length; ++i)
{
Expand All @@ -79,7 +80,7 @@ public static void InvokeCallbacksSafe<TValue1, TValue2>(ref CallbackArray<Actio
}
}
callbacks.UnlockForChanges();
Profiling.Profiler.EndSample();
marker.End();
}

public static bool InvokeCallbacksSafe_AnyCallbackReturnsTrue<TValue1, TValue2>(ref CallbackArray<Func<TValue1, TValue2, bool>> callbacks,
Expand Down