Skip to content
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

Jump to bottom

Update caches systems references to be actual caches and not an additional 0(n) lookup #10606

Merged
merged 4 commits into from
Jun 14, 2022
Merged

Update caches systems references to be actual caches and not an additional 0(n) lookup #10606

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2338a9f
Microsoft mandatory file (#10599)
microsoft-github-policy-service[bot] May 18, 2022
c864066
Update caches systems references to be actual caches and not an addit…
SimonDarksideJ May 21, 2022
a08ec66
Updated Reset cache implementations to also clear the individual cach…
SimonDarksideJ Jun 10, 2022
9f48d24
Merge branch 'releases/2.8.1' into feature/resolve-service-cache
keveleigh Jun 14, 2022
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
95 changes: 83 additions & 12 deletions Assets/MRTK/Core/Utilities/CoreServices.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,64 +20,94 @@ namespace Microsoft.MixedReality.Toolkit
/// </summary>
public static class CoreServices
{
private static IMixedRealityBoundarySystem boundarySystem;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We investigated this approach a while back (CC @MaxWang-MS) and decided against it, but I don't remember the exact reason now...

If we go this approach now, we'll probably want to clear these new caches in ResetCacheReference and ResetCacheReferences

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look to update it and clear the caches from that call. Although I am concerned why you would even need to reset the caches as all the systems listed in this function are transient, meaning they should never be cleared for the lifetime of the runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated as per your request @keveleigh

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the systems listed in this function are transient, meaning they should never be cleared for the lifetime of the runtime.

Just to make sure we're talking about the same thing, my understanding of "transient" and "should never be cleared for the lifetime of the runtime" are two different things: the first is short-lived, and the other sounds more long-lived.

Either way though, we don't assert that MRTK services are specifically one or the other! Devs may swap profiles at runtime, leading to these cached references potentially going stale. The cache clearing logic all happens in MixedRealityToolkit.cs when the profile is changed and the services are torn down and spun back up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also going to investigate cherry-picking this to 2.8.1. I think this will help with a selection of perf issues I've been strategically targeting. Thanks!


/// <summary>
/// Cached reference to the active instance of the boundary system.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealityBoundarySystem BoundarySystem => GetService<IMixedRealityBoundarySystem>();
public static IMixedRealityBoundarySystem BoundarySystem => boundarySystem ?? (boundarySystem = GetService<IMixedRealityBoundarySystem>());

private static IMixedRealityCameraSystem cameraSystem;

/// <summary>
/// Cached reference to the active instance of the camera system.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealityCameraSystem CameraSystem => GetService<IMixedRealityCameraSystem>();
public static IMixedRealityCameraSystem CameraSystem => cameraSystem ?? (cameraSystem = GetService<IMixedRealityCameraSystem>());

private static IMixedRealityDiagnosticsSystem diagnosticsSystem;

/// <summary>
/// Cached reference to the active instance of the diagnostics system.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealityDiagnosticsSystem DiagnosticsSystem => GetService<IMixedRealityDiagnosticsSystem>();
public static IMixedRealityDiagnosticsSystem DiagnosticsSystem => diagnosticsSystem ?? (diagnosticsSystem = GetService<IMixedRealityDiagnosticsSystem>());

private static IMixedRealityFocusProvider focusProvider;

/// <summary>
/// Cached reference to the active instance of the focus provider.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealityFocusProvider FocusProvider => GetService<IMixedRealityFocusProvider>();
public static IMixedRealityFocusProvider FocusProvider => focusProvider ?? (focusProvider = GetService<IMixedRealityFocusProvider>());

private static IMixedRealityInputSystem inputSystem;

/// <summary>
/// Cached reference to the active instance of the input system.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealityInputSystem InputSystem => GetService<IMixedRealityInputSystem>();
public static IMixedRealityInputSystem InputSystem => inputSystem ?? (inputSystem = GetService<IMixedRealityInputSystem>());

private static IMixedRealityRaycastProvider raycastProvider;

/// <summary>
/// Cached reference to the active instance of the raycast provider.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealityRaycastProvider RaycastProvider => GetService<IMixedRealityRaycastProvider>();
public static IMixedRealityRaycastProvider RaycastProvider => raycastProvider ?? (raycastProvider = GetService<IMixedRealityRaycastProvider>());

private static IMixedRealitySceneSystem sceneSystem;

/// <summary>
/// Cached reference to the active instance of the scene system.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealitySceneSystem SceneSystem => GetService<IMixedRealitySceneSystem>();
public static IMixedRealitySceneSystem SceneSystem => sceneSystem ?? (sceneSystem = GetService<IMixedRealitySceneSystem>());

private static IMixedRealitySpatialAwarenessSystem spatialAwarenessSystem;

/// <summary>
/// Cached reference to the active instance of the spatial awareness system.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealitySpatialAwarenessSystem SpatialAwarenessSystem => GetService<IMixedRealitySpatialAwarenessSystem>();
public static IMixedRealitySpatialAwarenessSystem SpatialAwarenessSystem => spatialAwarenessSystem ?? (spatialAwarenessSystem = GetService<IMixedRealitySpatialAwarenessSystem>());

private static IMixedRealityTeleportSystem teleportSystem;

/// <summary>
/// Cached reference to the active instance of the teleport system.
/// If system is destroyed, reference will be invalid. Please use ResetCacheReferences()
/// </summary>
public static IMixedRealityTeleportSystem TeleportSystem => GetService<IMixedRealityTeleportSystem>();
public static IMixedRealityTeleportSystem TeleportSystem => teleportSystem ?? (teleportSystem = GetService<IMixedRealityTeleportSystem>());

/// <summary>
/// Resets all cached system references to null
/// </summary>
public static void ResetCacheReferences() => serviceCache.Clear();
public static void ResetCacheReferences()
{
serviceCache.Clear();
boundarySystem = null;
cameraSystem = null;
diagnosticsSystem = null;
focusProvider = null;
inputSystem = null;
raycastProvider = null;
sceneSystem = null;
spatialAwarenessSystem = null;
teleportSystem = null;
}

/// <summary>
/// Clears the cache of the reference with key of given type if present and applicable
Expand All @@ -91,6 +121,7 @@ public static bool ResetCacheReference(Type serviceType)
if (serviceCache.ContainsKey(serviceType))
{
serviceCache.Remove(serviceType);
ResetCacheReferenceFromType(serviceType);
return true;
}
}
Expand All @@ -102,6 +133,46 @@ public static bool ResetCacheReference(Type serviceType)
return false;
}

private static void ResetCacheReferenceFromType(Type serviceType)
{
if (typeof(IMixedRealityBoundarySystem).IsAssignableFrom(serviceType))
{
boundarySystem = null;
}
if (typeof(IMixedRealityCameraSystem).IsAssignableFrom(serviceType))
{
cameraSystem = null;
}
if (typeof(IMixedRealityDiagnosticsSystem).IsAssignableFrom(serviceType))
{
diagnosticsSystem = null;
}
if (typeof(IMixedRealityFocusProvider).IsAssignableFrom(serviceType))
{
focusProvider = null;
}
if (typeof(IMixedRealityInputSystem).IsAssignableFrom(serviceType))
{
inputSystem = null;
}
if (typeof(IMixedRealityRaycastProvider).IsAssignableFrom(serviceType))
{
raycastProvider = null;
}
if (typeof(IMixedRealitySceneSystem).IsAssignableFrom(serviceType))
{
sceneSystem = null;
}
if (typeof(IMixedRealitySpatialAwarenessSystem).IsAssignableFrom(serviceType))
{
sceneSystem = null;
}
if (typeof(IMixedRealityTeleportSystem).IsAssignableFrom(serviceType))
{
teleportSystem = null;
}
}

/// <summary>
/// Gets first matching <see cref="Microsoft.MixedReality.Toolkit.Input.IMixedRealityInputDeviceManager"/> or extension thereof for CoreServices.InputSystem
/// </summary>
Expand Down Expand Up @@ -147,7 +218,7 @@ public static T GetDataProvider<T>(IMixedRealityService service) where T : IMixe
// We do not want to keep a service around so use WeakReference
private static readonly Dictionary<Type, WeakReference<IMixedRealityService>> serviceCache = new Dictionary<Type, WeakReference<IMixedRealityService>>();

private static T GetService<T>() where T : IMixedRealityService
private static T GetService<T>() where T : IMixedRealityService
{
Type serviceType = typeof(T);

Expand Down Expand Up @@ -176,4 +247,4 @@ private static T GetService<T>() where T : IMixedRealityService
return service;
}
}
}
}