Broken GPU Instancing - Performance Degradation Upgrading From MRTK2 Standard Shader

[Schroedingers-Cat]

Upgrading From MRTK2's Standard Shader Breaks GPU Instancing And Causes Performance Degradation

After upgrading our project to MRTK3, we observed our app's framerate dropping down to 30-40 fps in a scene with 520 instances of 4 variants of low-vertex-count models, sharing the same opaque material with GPU Instancing enabled. We identified the source of the issue being the new GraphicsTools/Standard (GTS) shader from MRTK3 Graphics Tools. Switching back to MixedRealityToolkit/Standard (MRTS) shader eliminated the issue and brought us back up to 60 FPS.

We investigated this with RenderDoc and the difference between the two seems to be that GTS is ignoring its Material's GPU Instancing setting causing a much higher event count per frame than when using MRTS. RenderDoc.zip

With the MRTS shader, instanced objects are drawn with very few DrawIndexedInstanced(IndexCountPerInstance, InstanceCount) calls, often with the InstanceCount argument being tens or hundreds:
But with the GTS shader, DrawIndexedInstanced always gets called with only 1 as InstanceCount argument:

To reproduce

Steps to reproduce the behavior:

1. Download, extract and open this project MRTK3 Shader Performance Degradation.zip
2. Open the scene MRTK3 Shader Performance Issue.unity which has meshes all using the same material HäuserColourMap
3. Verify that this material uses the Graphics Tools/Standard shader (from this repository) and "GPU Instancing" is enabled
4. Run the scene and verify that the Stats window shows Saved by batching: 0
5. Use RenderDoc to verify that DrawIndexedInstanced is called with an instance argument of 1 leading to a high amount of render events
6. Stop play mode
7. Deploy a UWP build to HL2 and verify it runs at 30-40 fps
8. On the HäuserColourMap, switch its shader to Mixed Reality Toolkit/Standard (from the MRTK2)
9. Run the scene and verify that the Stats window shows Saved by batching ~1500
10. Use RenderDoc to verify that DrawIndexedInstanced is called with an instance argument of tens or hundreds, leading to only a few render events
11. Deploy a UWP build to HL2 and verify it runs at 60fps

Expected behavior

We expected the new default shader to not break GPU instancing (if enabled in its material settings) and to not degrade performance.

Setup

• Unity Version 2022.3.59f1
• Graphics Tools Version v0.8.1

Target platform

• HoloLens 2

Additional context

It's worth noting that inspecting the new MeshInstancer example scenes with RenderDoc seems like the InstanceCount argument of the DrawIndexedInstanced call is considerably higher than one there. However, this used to work automatically with the MRTK2 shader which allowed our designers to just place content using Unity's tools.

[Cameron-Micka]

Hello @Schroedingers-Cat, thank you for the detailed bug report! I will try to look into this as soon as possible. One possible clue I see in your RenderDoc screenshot is your project is using Unity's SRP batcher:

Generally speaking the SRP batcher is a good thing (a performance booster) for most scenes (more draw calls, but less state switching) - but it's a performance loss for some types of scene setups.

Out of curiosity if you disable the SRP batcher (you can even do it dynamically via GraphicsSettings.useScriptableRenderPipelineBatching = false) does instancing improve? Or, more interesting does performance improve or get worse?

[Schroedingers-Cat]

Out of curiosity if you disable the SRP batcher (you can even do it dynamically via GraphicsSettings.useScriptableRenderPipelineBatching = false) does instancing improve? Or, more interesting does performance improve or get worse?

@Cameron-Micka when disabling the SRP batcher we get the same performance results as with the MRTK2 shader MixedRealityToolkit/Standard on the HL2 device.

[Cameron-Micka]

Thanks for trying that out. That makes sense because the MRTK2 Shader does not support the SRP Batcher. So materials that use it will default to the normal (non-SRP Batcher) behavior even when the SRP Batcher is enabled.

Given this info I will close this issue out. Each project has slightly different requirements, so I'd recommend your project disable the SRP Batcher if it is a performance win. Please let me know if you would like to reopen this!

[Schroedingers-Cat]

Given this info I will close this issue out. Each project has slightly different requirements, so I'd recommend your project disable the SRP Batcher if it is a performance win.

I referred to the demo project I uploaded when I said that disabling the SRP batcher gives the same performance results as with the MRTK2 shader. The demo project is reduced to the performance degradation we identified in our scene after updating to the MRTK3 shaders.

Our actual project does render a lot of other things in the same scene. In that project, disabling SRP batcher helps a bit gaining back some of our original MRTK2 performance but it's still worse than that. This is very likely because a lot of the other objects with the MRTK3 shaders that didn't use GPU instancing with the MRTK2 shaders will now suffer from disabling SRP batcher.

Given this info I will close this issue out. Each project has slightly different requirements, so I'd recommend your project disable the SRP Batcher if it is a performance win. Please let me know if you would like to reopen this!

I think this issue should be reopened because automatic GPU instancing used to work and now it doesn't, similar to how other MRTK2 features still aren't available in MRTK3 - it's something that should be worked towards bringing back (unless there was a good reason to explicitly remove it?). Apart from that, the performance degradation by removing automatic GPU instancing isn't mentioned in any of the MRTK docs. I think users upgrading to MRTK3 and the GraphicsTools should be made aware of this before they upgrade.

Why would the new shader stop automatically using GPU instancing in such a text book example (same mesh, same material), esp. when it's enabled in the material settings?

[Cameron-Micka]

I think this issue should be reopened because automatic GPU instancing used to work and now it doesn't, similar to how other MRTK2 features still aren't available in MRTK3 - it's something that should be worked towards bringing back (unless there was a good reason to explicitly remove it?)

Good point, I wish this is something we had more control over, but unfortunately this is a design consideration made by Unity and not Graphics Tools. Here are more details: GPU instancing isn’t compatible with the SRP Batcher. The SRP Batcher takes priority over GPU instancing. If a GameObject is compatible with the SRP Batcher, Unity uses the SRP Batcher to render it, not GPU instancing.

Why would the new shader stop automatically using GPU instancing in such a text book example (same mesh, same material), esp. when it's enabled in the material settings?

I proposed a similar question to Unity and they were empathic - the SRP Batcher isn't ideal for all scenarios but in their findings was a net win for the "average" scenario. One thing we could try is add an extra checkbox to the Graphics Tools/Standard shader that "disables SRP batcher compatibility". By default, this checkbox would be off, but you could manually enable this functionality for materials that would benefit from being rendered with instancing. Would this be helpful for your scenario?

[Schroedingers-Cat]

Thanks for the details, we were working with 2019's built-in render pipeline for years and weren't aware of this!

One thing we could try is add an extra checkbox to the Graphics Tools/Standard shader that "disables SRP batcher compatibility". By default, this checkbox would be off, but you could manually enable this functionality for materials that would benefit from being rendered with instancing. Would this be helpful for your scenario?

Sounds great! If it's possible, the shader editor code could check for the "GPU Instancing" setting and automatically set the extra checkbox. The checkbox could even become an internal property of the shader.

But a simple checkbox for manually disabling SPR on a per-material setting would be enough for scenarios like ours.

[Cameron-Micka]

Wonderful! I'll re-open this issue to track adding that feature.

[Schroedingers-Cat]

@Cameron-Micka Looked into adding a setting for what we discussed but things turned out to be more limiting in Unity than I initially hoped. Using a preprocessor define to create a non-SRP shader variant does not work because a shader variant cannot change if it's an SRP or a non-SRP shader. This is decided on a per-shader level by Unity.
The other option would be a thin wrapper that sets the proprocessor define and includes the main shader file. However, a .shader file cannot include another .shader file, so that option is also out.

This leaves us with the only option to create a copy of the GraphicsToolsStandard.shader and set the preprocessor define there to make the included GraphicsToolsStandardInput.hsls compile for non-SRP. Since the GraphicsToolsStandard.shader is already quite big, this would create a lot of redundant code. So our best option here is to automate copying and patching the file to avoid maintaining two very similar files.

My question here is: would you accept a PR that adds a small CBUFFER_START/END section depending on a preprocessor define to GraphicsToolsStandardInput.hlsl so it can compile in an SRP and a non-SRP context and a small script (which language - powershell, c#/editor assetpostprocessor?) that automates copying and patching a full non-SRP .shader file?

[Cameron-Micka]

Thanks for tackling this @Schroedingers-Cat. I see what you mean about having to maintain another shader (we already have to do this with GraphicsToolsStandard and GraphicsToolsStandardCanvas so it would be nice to avoid having to maintain a third).

I think your solution would work. I'd prefer a c#/editor assetpostprocessor since it's more cross platform. Also you might need to be careful to handle non-mutable package situations. MRTK2 did something like this to support URP before Unity had the PackageRequirements shader lab syntax.

Using a preprocessor define to create a non-SRP shader variant does not work because a shader variant cannot change if it's an SRP or a non-SRP shader. This is decided on a per-shader level by Unity.

This is too bad. Just a random thought, I wonder if you look though the URP source if there is some function (even an internal one we can reflect) to force SRP batcher compatibility on/off per shader?