View Issue Details

IDProjectCategoryView StatusLast Update
0001587NoesisGUIUnity3Dpublic2019-12-30 12:19
ReporterstonstadAssigned Tojsantos 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version2.2.5 
Target Version2.2.6Fixed in Version2.2.6 
Summary0001587: NoesisView Garbage Generation Scenario
DescriptionI tracked down the source of a 8 kb per frame garbage allocation to NoesisView.cs. It turns out the cause is something one could argue is developer error, but it is *definitely* a pitfall that should be avoided. These empty try/catches in Update() are dangerous to the extreme for GC allocations and they can be avoided:

        try { if (Input.GetAxis("Noesis_Vertical") > 0.5f) gamepadButtons |= GamepadButtons.Up; } catch (Exception) {}
        try { if (Input.GetAxis("Noesis_Vertical") < -0.5f) gamepadButtons |= GamepadButtons.Down; } catch (Exception) {}

        try { if (Input.GetAxis("Noesis_Horizontal") > 0.5f) gamepadButtons |= GamepadButtons.Right; } catch (Exception) {}
        try { if (Input.GetAxis("Noesis_Horizontal") < -0.5f) gamepadButtons |= GamepadButtons.Left; } catch (Exception) {}

        try { if (Input.GetButton("Noesis_Accept")) gamepadButtons |= GamepadButtons.Accept; } catch (Exception) {}
        try { if (Input.GetButton("Noesis_Cancel")) gamepadButtons |= GamepadButtons.Cancel; } catch (Exception) {}
        try { if (Input.GetButton("Noesis_Menu")) gamepadButtons |= GamepadButtons.Menu; } catch (Exception) {}
        try { if (Input.GetButton("Noesis_View")) gamepadButtons |= GamepadButtons.View; } catch (Exception) {}
        try { if (Input.GetButton("Noesis_PageLeft")) gamepadButtons |= GamepadButtons.PageLeft; } catch (Exception) {}
        try { if (Input.GetButton("Noesis_PageRight")) gamepadButtons |= GamepadButtons.PageRight; } catch (Exception) {}
        try { if (Input.GetAxis("Noesis_PageUp") > 0.5f) gamepadButtons |= GamepadButtons.PageUp; } catch (Exception) {}
        try { if (Input.GetAxis("Noesis_PageDown") > 0.5f) gamepadButtons |= GamepadButtons.PageDown; } catch (Exception) {}

        try { float v = Input.GetAxisRaw("Noesis_Scroll"); if (Math.Abs(v) > 0.05f) _uiView.Scroll(v); } catch (Exception) {}
        try { float v = Input.GetAxisRaw("Noesis_HScroll"); if (Math.Abs(v) > 0.05f) _uiView.HScroll(v); } catch (Exception) {}

A developer may choose to remove any one of these input labels for any number of reasons and Noesis will silently allocate multiple KB of garbage for each frame rendered. These thrown exceptions are caught silently. Since Unity does NOT report exceptions while a debugger is attached it is highly likely a developer will never know this is happening. The best case scenario is that they will just think, "Noesis creates garbage," when it usually doesn't.

As a proposed solution, NoesisView could check for the existence of these inputs on Awake() and then disable reading them in Update() if they were not previously defined. This design completely eliminates the need for try/catches in the Update loop and prevents possible GC allocation. This has the added benefit of allowing developers to eliminate Noesis inputs as they may find necessary to accommodate gameplay controls.


TagsNo tags attached.
PlatformAny

Activities

stonstad

stonstad

2019-11-24 14:47

reporter  

GC Allocations.PNG (13,372 bytes)
GC Allocations.PNG (13,372 bytes)
stonstad

stonstad

2019-11-24 14:53

reporter   ~0006006

NoesisView.UpdateMouse() also has an empty/try catch that is capable of allocating garbage. Easily avoided by checking for existence of input on Awake(). Or at minimum just throw an exception so we are defensive here and developers won't be unaware that per frame garbage is being allocated.
jsantos

jsantos

2019-11-25 18:11

manager   ~0006009

We are aware of this, and that's the reason we added a checkbox in the View to disable input from gamepads. But, you are totally right, it is better to detect each mapping on Awake() and then use that information in every frame.

Thanks for this ticket!
stonstad

stonstad

2019-11-25 23:50

reporter   ~0006010

If you'd like a PR I'd be happy to share. I'm thinking Hashset<string> would have negligible impact...
jsantos

jsantos

2019-11-26 13:56

manager   ~0006011

I think I prefer a bunch of booleans but I am not sure. Thanks for the offer. I will write again here whenever I start working on it.
stonstad

stonstad

2019-12-16 18:11

reporter   ~0006037

One of the gotchas to any design that minimizes the exception throwing invocations -- is that at some point in the future a controller may be connected. The validation logic would need to rerun to allow the connected controller to function.

I can't speak for other developers, but it would be easy for me to call a Refresh method on a view to rescan for valid inputs once the game detects a controller. You could also check for controller connected events -- but my experience is that those workflows are historically buggy in Unity, and it may be easier to offload that to the developer.
jsantos

jsantos

2019-12-16 18:21

manager   ~0006038

Even if the controller is not connected, Input.GetButton() shouldn't throw an exception because as far as I understand the exception is raised when the mapping is not in the list.

So the check done at init time should be valid forever, am I right?
stonstad

stonstad

2019-12-16 18:48

reporter   ~0006039

Oy, you're right!
jsantos

jsantos

2019-12-30 12:19

manager   ~0006051

Fixed in the imminent 2.2.6. I have attached the new NoesisView.cs
jsantos

jsantos

2019-12-30 12:19

manager  

NoesisView.cs (44,122 bytes)

Issue History

Date Modified Username Field Change
2019-11-24 14:47 stonstad New Issue
2019-11-24 14:47 stonstad File Added: GC Allocations.PNG
2019-11-24 14:49 stonstad Description Updated View Revisions
2019-11-24 14:53 stonstad Note Added: 0006006
2019-11-25 18:08 jsantos Assigned To => jsantos
2019-11-25 18:08 jsantos Status new => assigned
2019-11-25 18:11 jsantos Note Added: 0006009
2019-11-25 18:18 jsantos Target Version => 2.2.6
2019-11-25 23:50 stonstad Note Added: 0006010
2019-11-26 13:56 jsantos Note Added: 0006011
2019-12-16 18:11 stonstad Note Added: 0006037
2019-12-16 18:21 jsantos Note Added: 0006038
2019-12-16 18:21 jsantos Status assigned => feedback
2019-12-16 18:48 stonstad Note Added: 0006039
2019-12-16 18:48 stonstad Status feedback => assigned
2019-12-30 12:19 jsantos Status assigned => resolved
2019-12-30 12:19 jsantos Resolution open => fixed
2019-12-30 12:19 jsantos Fixed in Version => 2.2.6
2019-12-30 12:19 jsantos Note Added: 0006051
2019-12-30 12:19 jsantos File Added: NoesisView.cs