Suggestion / Idea ObiUpdater needlessly allocates every frame

[josemendez]

I've been having performance issues with Obi Cloth, and have been spending lots of time in the profiler to get a handle on how to improve things.

I've noticed that Obi allocates quite a lot of memory each frame - in my scene with 6 clothed characters, `ObiLateFixedUpdater` is allocating ~2.5 kb each frame. At least some of this is very easily eliminated.

`ObiUpdater.BeginStep` and `ObiUpdater.Substep` both allocate a new temporary List each time they're called. This list can be moved into a member variable to avoid the repeated allocation:

Code:

private List<IObiJobHandle> m_JobHandles = new List<IObiJobHandle>();
protected void Substep(float stepDeltaTime, float substepDeltaTime, int index)
{
    using (m_SubstepPerfMarker.Auto())
    {
        // Kick off all solver jobs:
        foreach (ObiSolver solver in solvers)
            if (solver != null)
                this.m_JobHandles.Add(solver.Substep(stepDeltaTime, substepDeltaTime, index));

        // wait for all solver jobs to complete:
        foreach (IObiJobHandle handle in this.m_JobHandles)
            if (handle != null)
                handle.Complete();

        this.m_JobHandles.Clear();
    }
}

`ObiSolver.Update` also does some unnecessary allocation, because it calls `Mathf.Max(params float[])`, which allocates a temporary params array. (It also accesses `transform.lossyScale` - which I think involves a roundtrip out of managed code and into the engine - three times). I've made this minor tweak:

Code:

void Update()
{
    var lossyScale = transform.lossyScale;
    m_MaxScale = Mathf.Max(Mathf.Max(lossyScale.x, lossyScale.y), lossyScale.z);
}

These changes bring my allocations down to ~1.5kb per frame, which is a good start - but far from perfect!

I really like Obi, and I'm a bit surprised to see so much `GC.alloc` action every frame. I'd love to be able to do cloth simulation without any allocations... might there be a performance pass done in the future?

Cheers!

Hi there!

Thanks a lot for the input! One of these (the updater list<> as a class member had been already addressed in the development branch, but I wasn't aware of the other one (Mathf.Max allocating a temporary array for any more than 2 arguments ). Both improvements will be included.

Regularly I profile Obi and grab low-hanging fruit, however not all have been picked yet. Expect less garbage generation in upcoming releases.