Unity-UI-Extensions / com.unity.uiextensions

https://unity-ui-extensions.github.io/
1.22k stars 132 forks source link

UI Line Renderers cause GC allocations #271

Closed SimonDarksideJ closed 2 years ago

SimonDarksideJ commented 2 years ago

Issue created by Evgeniy as Bitbucket Issue #​271 on 2018.11.28 10:23. Basically subj. For example 20 4-point line renderers generate about 55kb of garbage every frame. Unity 2018.2.10. LineRendererGC.PNG

I tried using Line List mode, but i need control over each line's color, so its not an option.

SimonDarksideJ commented 2 years ago

On 2018.12.17 16:13, @SimonDarksideJ commented: Ouch, that is not good. Can you provide a sample scene which demonstrates this issue please?

Ideally though, it sounds like an additional requirement on the line renderer as the control isn't meant to be used repeatedly in the scene ideally.

SimonDarksideJ commented 2 years ago

On 2018.12.19 18:31, Evgeniy commented: Heres the repro: https://drive.google.com/file/d/1OsOhsDS9IFKyxA5jr5WuS6bLT9AFXpbE/view?usp=sharing

Upon further inspection i found that i have to recreate an array each time i want to update the the line points, maybe thats why it causes the GC, although the profiler doesnt point at my script, it points at some internal canvas stuff. If i just access the existing array and change points there, it wont update the graphic, even though the points are updated in the inspector. Ive tried using the Rebuild method, but it seems to do nothing.

SimonDarksideJ commented 2 years ago

On 2019.02.11 16:40, Evgeniy commented: Any updates on this?

SimonDarksideJ commented 2 years ago

On 2019.02.11 18:10, @SimonDarksideJ commented: Nothing further yet, still working on updates atm.

SimonDarksideJ commented 2 years ago

On 2019.03.07 20:07, @SimonDarksideJ commented: Ok, so after reviewing, for the next point update (as it seems the old asset hasn't been updated in a while :S) I'm going to add a variant of the LineRenderer called LineRenderer List.

This will be an implementation of the same Linerenderer but using a List instead of an array. This will allow some A/B testing against the performance of the line renderer in either mode.

Hopefully you'll be on board to test this new variant @CustomPhase ?

SimonDarksideJ commented 2 years ago

On 2019.03.10 13:07, Evgeniy commented: @ddreaper sure thing!

SimonDarksideJ commented 2 years ago

On 2020.03.13 09:49, Luca Heft commented: Im having the same problem 1.4MB of garbage (i have a lot of realtime graphs). When turning on Deep Profiler you can see the problem lies within UILineRenderer.OnPopulateMesh().

@Evgeniy you can force your LineRenderer to update by calling linerenderer.SetAllDirty()

@{557058:da9b1be2-6172-44a5-a085-cae5d30eda9e} Any update on this

SimonDarksideJ commented 2 years ago

On 2020.03.15 14:46, @SimonDarksideJ commented: Still investigating and trying to narrow down a resolution

SimonDarksideJ commented 2 years ago

On 2020.03.15 16:41, @SimonDarksideJ commented: Investigating today @{5e5e154208e2a40c9386425c} using the sample repo above in 2018.4, and I’m not seeing barely any allocations. Going to leave it running a few hours and see what the story is.
Currently it’s been running an hour with no run away memory usage and sitting steadily at a 68k GC alloc range, which is fair based on the sample scene.

Even running outside the editor, the runtime memory, including the player overhead is 80mb, with only 9mb in the GC alloc stack.

SO this looks to have been a UI bug in 2018.2, the version the project was created in.

Also going to test in the latest 2019 build too.

SimonDarksideJ commented 2 years ago

On 2020.03.15 17:29, @SimonDarksideJ commented: Right, been through this for the past few hours :smiley: , and any memory overflow was likely the result of a Unity bug in the canvas renderer.

Doing

points = new Vector2[4];

While not great in an Update loop, should not cause GC alloc as Vector2 is a value type. And so long as the array size doesn’t change, the GC would simply reuse the same memory segment for each new alloc

However, a better way (assuming the array of lines is static, would be to create the array in the root of the script and simply overwrite each element you wanted to change over time. There is no need to “new” it all the time if the data is the same. As the Array variable is the same, you don’t need to query the line renderer for it’s points, as the array is already the master copy of the lines.

As to the question of why the Line doesn’t redraw is you use the above method, is because the “Array” the LineRenderer is assigned has not changed. Yes, the values within the array have changed but not the array itself, hence it would not trigger the “SetAllDirty” when just changing the arrays contents. SO you need to call “SetAllDirty” manually if you are only updating the contents instead of assigning a new array.

For example:

            for (int i = 0; i<points.Length; i++)
            {
                points[i] = new Vector2(Random.Range(0, Screen.width), Random.Range(0, Screen.height));
            }
            lr.Points = points;
            lr.SetAllDirty();

Which updates the array and tells the canvas it needs to redraw.

Interestingly, in the latest 2019.3 build, it had me scratching my head for a while as the memory in the build, even in a standalone build did keep increasing. Until I just disabled the script and re-tested, only to find the memory STILL increased (had nothing to do with the script), seems Unity’s latest 2019.3 build has a memory leak in the player :open_mouth:

If there is another pattern someone wants me to test and report back on, let me know. But so far, I can’t find an issue in the latest builds for each Unity version. If you are seeing a leak, then I suggest updating your Unity version (but not 2019 :stuck_out_tongue: )

SimonDarksideJ commented 2 years ago

On 2020.03.16 09:24, Luca Heft commented: Thanks for your detailed research.

Is it expected that the LineRenderer generates a lot of garbage if you change its points?

I just upgraded to the latest version (2019.1) with Unity 2019.3 and im still seeing massive garbage generation.

Here is my script which will add a point to the LineRenderer 25 times a second (maybe thats overkill?)

public class LineChart : MonoBehaviour
{
    public float timeInView = 5f;
    public float minY = 0f;
    public float maxY = 1f;
    public float NextValue { get; set; }

    float updateRate = 1f / 25;
    int maxPoints;
    float time;
    UILineRenderer lineRenderer;

    void Awake()
    {
        maxPoints = Mathf.RoundToInt(timeInView / updateRate);
        lineRenderer = GetComponent<UILineRenderer>();
        lineRenderer.Points = new Vector2[0];
    }

    void Update()
    {
        time += Time.deltaTime;
        if (time > updateRate)
        {
            time -= updateRate;
            AddPoint();
        }
    }

    void AddPoint()
    {
        if (lineRenderer.Points.Length < maxPoints)
        {
            Vector2[] _points = new Vector2[lineRenderer.Points.Length + 1];
            for (int i = lineRenderer.Points.Length - 1; i >= 0; i--)
            {
                _points[i] = new Vector2(1 - 1f / (maxPoints - 1) * (lineRenderer.Points.Length - i), lineRenderer.Points[i].y);
            }
            _points[_points.Length - 1] = new Vector2(1, NextValue.Remap(minY, maxY, 0, 1, true));
            lineRenderer.Points = _points;
        }
        else
        {
            for (int i = 1; i < lineRenderer.Points.Length; i++)
            {
                lineRenderer.Points[i - 1].y = lineRenderer.Points[i].y;
            }
            lineRenderer.Points[lineRenderer.Points.Length - 1].y = NextValue.Remap(minY, maxY, 0, 1, true);
            lineRenderer.SetAllDirty();
        }
    }
}

SimonDarksideJ commented 2 years ago

On 2020.03.16 10:20, @SimonDarksideJ commented: OK, so one simple comment on your code, if you are adding points that frequently, then consider using a list and then using List.ToArray() rather than creating a new array every frame.
Alternatively, consider Array.Clone to a new empty array and then add the new item to the last slot.

Will check out your sample

SimonDarksideJ commented 2 years ago

On 2020.03.16 10:26, Luca Heft commented: I see, new array only gets created until maxPoints are reached. Then the array from the line renderer will be reused (if/else statement in AddPoint). Not sure though if thats legitimate.

SimonDarksideJ commented 2 years ago

On 2020.03.16 10:30, @SimonDarksideJ commented: Yes, that is also another approach, create an initial array large enough and only expand when you reach capacity. Although be warned, doing so will see GC spikes when you extend, so keep that in mind.

SimonDarksideJ commented 2 years ago

On 2020.03.16 12:03, @SimonDarksideJ commented: Commented on the other thread, you can simplify the AddPoints method to the following:

    List<Vector2> _points = new List<Vector2>();

    public void AddPointNormal(float value)
    {
        _points.Add(new Vector2(1, value));
        lineRenderer.Points = _points.ToArray();
        lineRenderer.SetAllDirty();

    }

But it’s still better for performance to use an array with a max bounds (Vector2[20000]) and simply recycle its contents. Be aware, extending an array (which you should use Array.Clone) is expensive and exponential

SimonDarksideJ commented 2 years ago

On 2020.03.16 12:43, Luca Heft commented: This wont help with the garbage allocation though, as the most garbage is generated in the PopulateMesh method of the UILineRenderer. Now im no expert on GC and perhaps the UILineRenderer is not suitable for real time graphs like i do them :smiley:

SimonDarksideJ commented 2 years ago

On 2020.03.16 13:52, @SimonDarksideJ commented: As I pointed out above, the LineRenderer itself doesn’t cause GC allocations, it’s purely how you manage the data you are copying to the renderer.

Might look at ways where this can accept a byref reference, so hosting the array externally to the control, but in the past that caused more issues than it solved.

Unless there are any other ideas.

I was also tinkering with using a List as the array, however, that incurs the same “ToList” overhead as the VBO (UI Graphic) only works with Vector2 arrays for the vertices.

SimonDarksideJ commented 2 years ago

On 2020.03.16 14:39, Luca Heft commented: Ok so i tested again and i still get ton of GC allocs in UILineRenderer. If i dont call SetAllDirty() the GC allocs vanish.

Call SetAllDirty():

for (int i = 1; i < lineRenderer.Points.Length; i++)
{
    lineRenderer.Points[i - 1].y = lineRenderer.Points[i].y;
}
lineRenderer.Points[lineRenderer.Points.Length - 1].y = NextValue.Remap(minY, maxY, 0, 1, true);
lineRenderer.SetAllDirty();

Dont call SetAllDirty():

for (int i = 1; i < lineRenderer.Points.Length; i++)
{
    lineRenderer.Points[i - 1].y = lineRenderer.Points[i].y;
}
lineRenderer.Points[lineRenderer.Points.Length - 1].y = NextValue.Remap(minY, maxY, 0, 1, true);
//lineRenderer.SetAllDirty();

Overall Garbage drops significantly from 1.4MB to 11.1KB

SimonDarksideJ commented 2 years ago

On 2020.03.16 14:57, @SimonDarksideJ commented: You probably don’t get garbage because it’s not updating the UI. The only way around that is to replace the Points array with a new instance of an array and SetAllDirty is called by the LineRenderer control

We need to “Dirty” the canvas to force a redraw. You can improve how much you redraw by having the line renderer on its own canvas if you wish, standard UI trick.

SimonDarksideJ commented 2 years ago

On 2020.03.16 15:01, Luca Heft commented: Yup its not updating the UI. Welp i guess i have to live with that^^
Thanks for your help :slight_smile:

SimonDarksideJ commented 2 years ago

On 2020.03.16 15:07, @SimonDarksideJ commented: Ok, I’ve really gone back to nitty gritty and been looking at some of the base functionality for drawing primitives. A lot of that code was defined by the base Unity UI code. However, reviewing it with fresher eyes, there are some options for possible improvements, so I’m going to try some tinkering.

Granted, this may not work, but I’ll see what I can do to improve things. The biggest blocker is Unity’s UI system, which was not built for dynamism, like the 3D engine is.

SimonDarksideJ commented 2 years ago

On 2020.03.16 15:11, Luca Heft commented: Alright, hopefully the new UI system will handle things better :smiley:

Hit me up if you found some improvements.

SimonDarksideJ commented 2 years ago

On 2020.03.16 15:38, @SimonDarksideJ commented: #ROFL, the New UI system will be better. UIElements will be good for web developers :smiley:

While I’m looking at other improvements, I took your original script and made a few performance optimisations

public class AddPoint : MonoBehaviour
{
    UILineRenderer lineRenderer;
    int maxPoints = 1000;
    int count = 0;
    Vector2[] PointsArray = new Vector2[1000];

    void Start()
    {
        PointsArray[0] = Vector2.zero;
        lineRenderer = GetComponent<UILineRenderer>();
        lineRenderer.Points = PointsArray;
    }

    void Update()
    {
        if (++count < maxPoints)
        { 
            AddPointNormal((Mathf.Sin(Time.time * 4) + 1) / 2);
        }
    }

    public void AddPointNormal(float value)
    {
        for (int i = 1; i < count; i++)
        {
            lineRenderer.Points[i - 1].x = lineRenderer.Points[i].x;
            lineRenderer.Points[i - 1].y = lineRenderer.Points[i].y;
        }

        lineRenderer.Points[count].x = 1 - 1f / (maxPoints - 1) * count;
        lineRenderer.Points[count].y = value;
        lineRenderer.SetAllDirty();
    }
}

Hopefully, you can take something from that. This allocates only 1.7mb GC Alloc and doesn't grow

SimonDarksideJ commented 2 years ago

On 2020.03.16 15:54, Luca Heft commented: Your example doesnt work for me at all. I get some wierd results, but i see the point. Reuse the array, which i do when maxPoints are reached.

SimonDarksideJ commented 2 years ago

On 2020.03.16 16:10, @SimonDarksideJ commented: Hmm, here’s the full script, granted it’s hacky as I don’t reset the end point, but it’s just a demonstration on how to have an eternally extending array in the most performant way.

using UnityEngine;
using UnityEngine.UI.Extensions;

public class AddPoint : MonoBehaviour
{
    UILineRenderer lineRenderer;
    int maxPoints = 1000;
    int count = 0;
    Vector2[] PointsArray = new Vector2[1000];

    void Start()
    {
        PointsArray[0] = Vector2.zero;
        lineRenderer = GetComponent<UILineRenderer>();
        lineRenderer.Points = PointsArray;
    }

    void Update()
    {
        if (++count < maxPoints)
        { 
            AddPointNormal((Mathf.Sin(Time.time * 4) + 1) / 2);
        }
        else
        {
            maxPoints += 1000;
            PointsArray = new Vector2[maxPoints];
            lineRenderer.Points.CopyTo(PointsArray, 0);
            lineRenderer.Points = PointsArray;
            AddPointNormal((Mathf.Sin(Time.time * 4) + 1) / 2);
        }
    }

    public void AddPointNormal(float value)
    {
        for (int i = 1; i < count; i++)
        {
            lineRenderer.Points[i - 1].x = lineRenderer.Points[i].x;
            lineRenderer.Points[i - 1].y = lineRenderer.Points[i].y;
        }

        lineRenderer.Points[count].x = 1 - 1f / (maxPoints - 1) * count;
        lineRenderer.Points[count].y = value;
        lineRenderer.SetAllDirty();
    }
}

:smiley:

SimonDarksideJ commented 2 years ago

On 2020.03.16 16:17, Luca Heft commented: I just tested GC and its the same for both scripts :smiley:

SimonDarksideJ commented 2 years ago

On 2020.03.16 17:08, @SimonDarksideJ commented: Except it’s static and not climbing away, the only overhead is the size of your original array and it’s data, there shouldn’t be any creep.

Just spent a few hours looking for anything to scrape off, and can only shave it down slightly, so no real further improvement beyond what the UI Engine gives. So the key is to ensure you manage your code effectively, which I’m happy to help with and advise on.

SimonDarksideJ commented 2 years ago

On 2020.05.22 10:51, @SimonDarksideJ commented: Any progress @{5e5e154208e2a40c9386425c} ? I seem to get different results from different Unity versions

SimonDarksideJ commented 2 years ago

On 2020.05.26 08:01, Luca Heft commented: I did not test this in a while, because this isnt a critical problem for my application. I can live with that garbage :smiley:
Altough obviously i´d love to see the overall GC reduced.

SimonDarksideJ commented 2 years ago

On 2020.05.28 14:01, @SimonDarksideJ commented: Been through the actual code several times now and the GC seems to be caused by the underlying UI system, which is also dependant on Unity version.