Feature: gc problem

[chasinghope]

[crashkonijn]

Hi!

Thanks you for giving the package a go and reaching out!

Have you tried enabling deep profiling? The system calls the sensors and actions, which is usually where the GC issues are. Without deep profiling you won't see that.

[crashkonijn]

I just noticed that you're running the demo scene, the code in there is written for simplicity and not optimized for actual game code. They indeed produce a lot of GC (The system itself shouldn't)

[barocon]

I also recently noticed that the Agents property is GoapRunner.cs is generating a bit garbage every frame. Caching the array in Register function works to stop that. QueueCount seems to also generate 40 bytes of GC every frame. I haven't had time to test in a build yet. Could be less of a problem there.

[chasinghope]

Most GC is due to excessive reliance on linq. I think these should be optimized

[chasinghope]

I just noticed that you're running the demo scene, the code in there is written for simplicity and not optimized for actual game code. They indeed produce a lot of GC (The system itself shouldn't)

yes.I generated 1000 agents

[crashkonijn]

Allright, I'll try to remove some more Linq for the next major version. I've already simplified some interfacing which removed a lot of casting between types in that version.

A PR is always welcome as well, since my time is somewhat limited at the moment.

[crashkonijn]

I've already greatly reduced the amount of GC that is now being created for V3.

At least a large portion of the GC still left now comes from the (unoptimized) demo code.

I will have a look at writing unity tests that could hopefully test the amount of GC produced.

[laicasaane]

Unnecessary GC.Alloc(s) also happen in LocalTargetSensorBase.Sense(). Currently the method must always return new instance of ITarget. It would help if you allow modifying existing instances if we want to.

[crashkonijn]

Unnecessary GC.Alloc(s) also happen in LocalTargetSensorBase.Sense(). Currently the method must always return new instance of ITarget. It would help if you allow modifying existing instances if we want to.

Fair enough, I can adjust the provided ITarget implementations to allow for that.

That would mean that you'd have to keep track of these instances yourself, is that enough? Or would you expect me to provide the previous value again?

[laicasaane]

I think it's better to provide the existing instance of ITarget. And whenever a new instance is returned the existing one will be replaced.

For example:

public void Sense(IWorldData worldData, IActionReceiver agent, IComponentReference references)
{
    var existingTarget = worldData.GetTarget(this.Key);
    var newTarget = this.Sense(agent, references, existingTarget)
    
    if (existingTarget != newTarget)
    {
        worldData.SetTarget(this.Key, newTarget);
    }
}

public virtual ITarget Sense(IActionReceiver agent, IComponentReference references, ITarget existingTarget = null)
{
#pragma warning disable CS0618 // Type or member is obsolete
    return this.Sense(agent as IMonoAgent, references);
#pragma warning restore CS0618 // Type or member is obsolete
}

Second thought: The approach above has to constantly access the worldData which I find not necessary if this.Key won't change after creation. So it might be even better if you provide a mechanism so that we can create an instance of ITarget then add it into worldData only once.

[crashkonijn]

If you continue your second thought we might end up wanting to have a tickrate for sensors. The PlayerTarget probably only has to be provided once. Although I do like that idea I'm trying to get to some sort of finishing point for v3 right now. Perhaps we can implement such a system for v3.

For now I'll probably implement providing the target back to you, since that's an easy fix.

[crashkonijn]

Released the ITarget in sensors in 3.0.14-beta!

[crashkonijn]

Second thought: The approach above has to constantly access the worldData which I find not necessary if this.Key won't change after creation. So it might be even better if you provide a mechanism so that we can create an instance of ITarget then add it into worldData only once.

I had an idea for this last week and had a go at implementing it.

@laicasaane what do you think of the changes in #254 ?

[laicasaane]

Hi, it might take a while until I can test those changes. At the moment I'm working on something else. Will report to you when I have a chance.