googlevr / gvr-unity-sdk

Google VR SDK for Unity
http://developers.google.com/vr/unity/
Other
2.71k stars 1.09k forks source link

Memory leak when using GVR camera with Unity 5.3.6 #283

Closed GabeWeiss closed 8 years ago

GabeWeiss commented 8 years ago

We're using the 0.8 SDK with Unity 5.3.6. We noticed the memory increasing linearly in our project to the point where it was crashing our app on older phone hardware (iPhone 5 and Nexus 5 initially). When profiled in XCode we clearly saw, not a memory leak, but memory allocations that were never released by some Unity objects, most notably the canvas objects.

To test, we dropped a GVR camera into a completely empty Unity scene, and noted that we were still leaking around 0.01MB/s. Dropping a single canvas with a button and some text increased the leak to 0.04MB/s.

After some more investigation, we tied the allocations to these two methods in Unity:

UI::Canvas::EmitWorldGeometry(Camera*, unsigned short&, bool) /Users/builduser/buildslave/unity/build/Runtime/UI/Canvas.cpp:833

UI::Canvas::DrawIntermediateRenderer(UI::Batch&, Matrix4x4f, int, Camera*, unsigned short)/Users/builduser/buildslave/unity/build/Runtime/UI/Canvas.cpp:764

We know it's not our scene as the same scene, built for Oculus (with no GVR Camera) doesn't leak.

We spent a day trying to narrow down where the leak was coming from by trying to coax the gvr scripts to not create/invoke different calls.

What we’ve found is that with VRMode disabled there seems to be a tiny memory growth but it’s extremely small and at times didn’t realize it was happening, it’s only with long run times that you see it even a little (not enough I think that it would risk a crash before the user or OS would kill it). This lead us to think it perhaps had something to do with the dual cameras and the canvas, but we were able to build a version of a 2 camera setup with offsets and a world space canvas (without any GVR) and saw no memory growth at all, not even a tiny leak.

We then broke different pieces down, what we found is that the Stereo Controller seems to be what starts the issue. We gathered this because if we disable it from ever being created (in the GVRViewer) the leak drops to about what it was with VRMode Off. Now when you dig deep you realize the stereo controller really sort of manages everything. So that’s basically the top of the call stack.

So far we’ve tried to disable each and every function we could find that seemed to run each frame, and have still not been able to track down a specific source below the stereo controller that is causing the issue. We can only assume at this point that we’ve missed some method.

Any assistance or ways to stop what's happening would be much appreciated.

smdol commented 8 years ago

Oh wow, that's a horrible leak. Confirmed on 5.3.6.

However, I tried Unity 5.3.3 and I do not see the leak occurring. Can you try it too in order to confirm?

Unity had a major regression, starting in 5.3.4p2, with drawing uGUI Canvas objects into cameras that have a RenderTexture as the target, rather than the main screen. From 5.3.4p2 up till 5.3.6f1 Canvases did not render at all. Now it looks like they render again in 5.3.6, but with a bad memory leak.

Note that every Cardboard camera in VR Mode is using a RenderTexture as the target (for distortion correction) -- this would be why you don't see the problem whenever you disable VR Mode or StereoController. Oculus also does not draw into a RenderTexture.

If you confirm that 5.3.3 does not show the leak, the bug has to be reported to Unity.

GabeWeiss commented 8 years ago

As suggested, downgrading to 5.3.3 stops the leak entirely. We don't see any memory increase.

As an added note, this is not a "leak" in the traditional sense. If it were, we would have caught it long before. The memory is seemingly being allocated and held, rather than leaking. This is also why things like trying to force garbage collection in Unity doesn't have an effect on it either.

Do we have a link to the Unity filed bug we can reference here as well to keep all the information in one place?

smdol commented 8 years ago

The original Unity bugs filed about Canvases not rendering at all into RenderTextures were 790264, 799386 and 795880.

fkochdev commented 8 years ago

Can confirm that switching to Unity 5.3.3 worked for me. And it was the only way to get our app working without crashes due to memory issues.

We have faced continous growing memory allocation on iOS and Android with 5.3.6p1 as well as 5.4f1. Crazy.

Adam-VisualVocal commented 8 years ago

I see the same leak just running the HeadsetDemo straight out of the latest Cardboard SDK for Unity on Unity 5.3.6 and 5.4.0. I've let the Android build of the demo run for a few hours and the native heap has climbed all the way up to 450 megs and still rising. Thanks for the tip about downgrading to 5.3.3. I'll give that a try.

Adam-VisualVocal commented 8 years ago

I downgraded to 5.3.3 and the leaks are gone. Does anyone know if there is an active Unity bug to track this issue? We use a decent number of canvas objects in our VR scenes so these leaks are killing us. Thanks.

GabeWeiss commented 8 years ago

There is! It's here:

https://issuetracker.unity3d.com/issues/googlevr-canvas-are-allocating-memory-and-do-not-release-it-when-using-vr-cameras

sapanda commented 8 years ago

Anyone have a decent workaround? We're losing 8MB per second with this leak. Downgraded back to 5.3.4p1, but that leaves the app in an extremely unstable state. With an app store submission in 10 days, we're short on time to figure out an acceptable workaround.

One option: Toggle between VR and non-VR mode in a single frame. Right now we black out the frame and it's a horrible user experience since we have to do it every 10 seconds. We're exploring capturing a screenshot and using that as a texture for the frame.

Any other suggestions?

Ethan-VisualVocal commented 8 years ago

@sadpanda We've been doing a bulk of our cardboard dev on 5.3.4f1 (not p1) and haven't had any issues, stability or otherwise. But, our sample size is pretty small (only a handful of devices) -- what kind of stability issues are you having?

wpinaud commented 8 years ago

@Ethan-VisualVocal When we downgrade to 5.3.4p1 Unity was very unstable (crashes randomly on launch, crashes when trying to do a build...), so we choose to stay in 5.4.0f3, and we are still trying to find a workaround to this cardboard leaks. Here is a video of the leaks on iOS (the end of the video is when the app crashes). During the experiment I was just moving the camera around. cardboardleaks

smdol commented 8 years ago

The issue is not specific to Cardboard. It happens when Unity renders a world-space Canvas with any Camera targeting a RenderTexture, and that is the mechanism we use to implement the distortion correction.

A different UI system (NGUI, etc) likely will not have this problem, though porting your UI to that is not a pleasant thought either.

Ethan-VisualVocal commented 8 years ago

@wpinaud yeah with 5.4.0f3 we found you don't even have to move the camera or have any Canvas elements visible. Just set the phone on the desk staring at empty space and it will leak.

At that insane rate of memory leak, iOS will probably occasionally terminate your app for some users even with your 10 second workaround. We were having major issues even with a leak of only 0.5MB/s

I'd be inclined to either fix the crashes you're encountering on 5.3.4f1 or investigate if you can workaround the leak by poking at things in the GvrPostRender: https://github.com/googlevr/gvr-unity-sdk/blob/master/GoogleVR/Legacy/Scripts/Internal/GvrPostRender.cs#L91 (e.g. If you destroy and recreate the RenderTexture, does the memory get released?)

@smdol Thanks for the clarification! I knew it was a Unity issue, but the RenderTexture was just a suspicion, so good to know that seems to be the specific culprit.

sapanda commented 8 years ago

We've got a depressing workaround:

Periodically delete and re-add all canvases in a scene. Obviously this is risky and all references to Canvas elements are lost (fortunately we don't store references to Canvases in our app). We managed to get the leaks down from a couple MB/sec to 0.2MB/sec. Still not great and causes the iOS app to crash in about 8 to 10 minutes. But at least it's a lot better than before.

https://gist.github.com/sapanda/d231a38cde2cb3fa9caf332566bedc8b

Another caveat: Switching between VR and non-VR mode in the scene causes a crash.

mswf commented 8 years ago

@sapanda I actually derived a better workaround, from a tucked away forum post from a Unity developer. http://forum.unity3d.com/threads/cant-view-canvas-in-daydream-vr.421401/

This issue is fixed and will be released in patches for both 5.4 and 5.3, however if you are using the cardboard plugin you can work around the memory leak for now.

In stereoController.cs OnPreCull comment out cam.enabled = false. The issue is that if the camera is not active at the end of the frame its resources wont be freed. This problem will be resolved in Unity going forward.

This workaround causes the main camera to still render the entire scene. I found a slightly "better" workaround. In addition to commenting out 'cam.enabled = false', I added the following code there:

// EDIT: No don't do this, look below for a better workaround
cam.clearFlags = CameraClearFlags.SolidColor;
cam.backgroundColor = Color.black;
cam.cullingMask = 0;   

I also tried changing the viewport rect to be zero width and height, but this messes up the input events.

Now here's to hoping the real fix will be released sooner rather than later.

I also don't have access to my Unity account right now. Could somebody please be so kind to forward this workaround to the Unity bug thread?

EDIT: Okay so this does have side-effects after all. After the application loses focus the workaround settings applied to the main camera are applied to the individual eye cameras, making everything black. I will report back when I find where to properly prevent this from happening.

EDIT2: I created a more satisfying workaround: Added #define CARDBOARD_HACK at the top of the StereoController.cs
In void OnPreCull () changed cam.enabled to

#if CARDBOARD_HACK
#warning Due to a Unity bug, a worldspace canvas in a camera that renders to a RenderTexture allocates infinite memory. Uncomment this line when fixed.
            BlackOutMonoCamera();
#else
            cam.enabled = false;
#endif

In IEnumerator EndOfFrame (), changed cam.enabled to

#if CARDBOARD_HACK
                RestoreMonoCamera();
#else
                cam.enabled = true;
#endif

And finally added the following methods to the buttom of the file:

#if CARDBOARD_HACK
    private CameraClearFlags m_MonoCameraClearFlags;
    private Color m_MonoCameraBackgroundColor;
    private int m_MonoCameraCullingMask;

    private void BlackOutMonoCamera ()
    {
        m_MonoCameraClearFlags = cam.clearFlags;
        m_MonoCameraBackgroundColor = cam.backgroundColor;
        m_MonoCameraCullingMask = cam.cullingMask;

        cam.clearFlags = CameraClearFlags.SolidColor;
        cam.backgroundColor = Color.black;
        cam.cullingMask = 0;
    }

    private void RestoreMonoCamera ()
    {
        cam.clearFlags = m_MonoCameraClearFlags;
        cam.backgroundColor = m_MonoCameraBackgroundColor;
        cam.cullingMask = m_MonoCameraCullingMask;
    }
#endif

This is of course still dirty as can be. If I run into other issues because of this hack I will report back here. EDIT3: Thanks Willisp for spreading the word! EDIT4: I remembered that I could share the modifications as a gist: https://gist.github.com/mswf/7e7269e2dad6085aef6b455078e69559

smdol commented 8 years ago

Hmm. The camera is re-enabled at the end of the frame by the coroutine. Perhaps that is too late, despite the wording of the forum post.