Citrus-Engine icon indicating copy to clipboard operation
Citrus-Engine copied to clipboard

Query regarding MediatorState - Objects Loop Performance

Open Snky opened this issue 9 years ago • 6 comments

How important is the implementation of: object = _objects.shift(); _objects.push(object);

rather than just: object = _objects[ i ];

( in the update function ) I thought I'd ask first, I've found the bottom one liner plus commenting out the push, at least 20% faster, but this could come off me being really silly, if you have a really important reason for sorting the objects.. like perhaps, groups do not work properly from the one liner way.

I haven't tested killing yet, but yeah, just curious about the implementation really, and it's purpose that I may not be seeing.

Snky avatar Jul 29 '15 21:07 Snky

I think I did the modification of that loop while working with killing objects and adding that "removeImmediately" function and so on. I push it back in only if the object is not killed. and I do shift/push so that objects are kept in order - order matters with physics. things used to be spliced I think when killed.

There might be other implications such as one object killing or removing an object immediately inside its update function

I guess with the new insertAt() removeAt() though, which I have not tried yet, the entire MediatorState update loop could change completely.

Feel free to suggest an entire new loop though that we could discuss though, We didn't focus on that crucial part of the engine for a while obviously

gsynuh avatar Jul 29 '15 21:07 gsynuh

Understood, I don't see the .removeAt function on the _objects vector o0.. but I do on the garbage array, was this not added to both according to the release notes of AIR 19?

Snky avatar Jul 29 '15 22:07 Snky

Right, unfortunately, I'm not as talented as the team, or my friend that added a couple extra lines to what I was suggesting, and the end result was this:

        var object:CitrusObject;

        for (var i:uint = 0; i < _numObjects; ++i) { //run through objects from 'left' to 'right'

            object = _objects[ i ];

            if (object.kill){

                _garbage.push(object); // push object to garbage
                            _objects.splice(i,1)
                            --i
                            --_numObjects; //+TailBit: oh, wait.. then one also have to lower the max amount of objects aswell

            }else {

                if (object.updateCallEnabled)
                    object.update(timeDelta);
            }
        }

I may of lied about how the garbage collection worked to him, can you answer his question? ^^ TailBit: is the garbage empty before you do this? ( this being, the shifting and pushing, I'm assuming the answer is, not necessarily, but I said yes, this could change the code he gave me tbh, he never makes mistakes ) he's a genius, I want him to try flash again, I miss him.

"TailBit: in this I only cut them out of the object array when they are added to garbage, that means that the array only need to change on kill and can stay unchanged when they should remain alive"


The performance difference, is incredible, if this patch is doing the exact same thing, we're talking from 0.5fps with a potential halt to 17fps.. for 2000 objects

Snky avatar Jul 29 '15 22:07 Snky

I think this is what the code looked like before, with the splice.

anyway yes garbage is empty after update is done because of this line "while((garbageObject = _garbage.shift()) != null)" that will empty it as we removeImmediately garbageObject

gsynuh avatar Jul 29 '15 22:07 gsynuh

you can see here the old version of the update loop : https://github.com/DaVikingCode/Citrus-Engine/blob/825b50e1f87b134901bbf1e010ed75b61c5d38fa/src/citrus/core/MediatorState.as

gsynuh avatar Jul 29 '15 22:07 gsynuh

Oh and I remember now why the loop looks like that :

As you may see here, we use _numObjects for the loop for (var i:uint = 0; i < _numObjects; ++i) { _numObjects is succeptible to change as one object might destroy another in its update() function. _numObjects is therefore variable hence I could not access the object by index as doing so might skip an object (and that was the first result I got when trying to keep it in fact)

gsynuh avatar Jul 29 '15 22:07 gsynuh