Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return removed components to the component pool #58

Closed
sschmid opened this issue Jan 19, 2016 · 18 comments
Closed

Return removed components to the component pool #58

sschmid opened this issue Jan 19, 2016 · 18 comments

Comments

@sschmid
Copy link
Owner

sschmid commented Jan 19, 2016

As a performance optimization the Code Generator creates component pool for each generated component type (https://github.com/sschmid/Entitas-CSharp/blob/develop/Tests/Tests/Entitas.CodeGenerator/Fixtures/PersonComponent.cs#L15)

When you replace or remove components, the old component gets pushed to the component pool.

However, neither e.RemoveAllComponents() nor pool.DestroyEntity() will make use of the component pool and won't push them back.

That should be fixed...

@sschmid sschmid changed the title Return removed components to the component pool [TODO] Return removed components to the component pool Jan 19, 2016
@sschmid sschmid changed the title [TODO] Return removed components to the component pool Return removed components to the component pool Jan 19, 2016
@sschmid
Copy link
Owner Author

sschmid commented Jan 19, 2016

Once we add a component using a generated method like e.AddHealth(42) we could add an event handler for OnComponentRemoved. This handler could check if the component index is the same and push the component to the component pool. This way it would work for both cases e.RemoveAllComponents() and pool.DestroyEntity()

What do you think?
I also have unity project for performance testing where I could check if the added event handler has a negative effect on the performance.

@sschmid
Copy link
Owner Author

sschmid commented Jan 19, 2016

Also, I think it's time to also add tests for the actual logic of the generated output, not only tests for the expected output of the generator. I'd like to add tests for the four component types (standard, flag, single std, single flag)

@sschmid
Copy link
Owner Author

sschmid commented Jan 19, 2016

First tests revealed it adds 312byte per component even with cached event handlers
entitas component pool

@sschmid
Copy link
Owner Author

sschmid commented Jan 19, 2016

Creating new components would be cheaper...

@trumpets
Copy link
Contributor

I see... The issue I stumbled upon, and thus discovering this intricacy, was that my object count was increasing a lot, I had a lot of entities which were being destroyed and created, and the GC was turning on once in ~10 secs and that was stuttering my game :/

I am tinkering with the code right now, trying to figure out another way...

@sschmid
Copy link
Owner Author

sschmid commented Jan 19, 2016

91%, to be precise :)
entitas new component

@sschmid
Copy link
Owner Author

sschmid commented Jan 19, 2016

Here's my test, if you're interested:

@sschmid
Copy link
Owner Author

sschmid commented Jan 19, 2016

using System.Collections.Generic;

namespace Entitas {
    public partial class Entity {
        public PositionComponent position { get { return (PositionComponent)GetComponent(ComponentIds.Position); } }

        public bool hasPosition { get { return HasComponent(ComponentIds.Position); } }

        static readonly Stack<PositionComponent> _positionComponentPool = new Stack<PositionComponent>();

        public static void ClearPositionComponentPool() {
            _positionComponentPool.Clear();
        }

        EntityChanged cachedOnComponentRemoved {
            get {
                if (_cachedOnComponentRemoved == null) {
                    _cachedOnComponentRemoved = onComponentRemoved;
                }

                return _cachedOnComponentRemoved;
            }
        }

        EntityChanged _cachedOnComponentRemoved;

        public Entity AddPosition(float newX, float newY, float newZ) {
            OnComponentRemoved += cachedOnComponentRemoved;
            var component = _positionComponentPool.Count > 0 ? _positionComponentPool.Pop() : new PositionComponent();
            component.x = newX;
            component.y = newY;
            component.z = newZ;
            return AddComponent(ComponentIds.Position, component);
        }

        public Entity ReplacePosition(float newX, float newY, float newZ) {
//            var previousComponent = hasPosition ? position : null;
            var component = _positionComponentPool.Count > 0 ? _positionComponentPool.Pop() : new PositionComponent();
            component.x = newX;
            component.y = newY;
            component.z = newZ;
            ReplaceComponent(ComponentIds.Position, component);
//            if (previousComponent != null) {
//                _positionComponentPool.Push(previousComponent);
//            }
            return this;
        }

        public Entity RemovePosition() {
//            var component = position;
            RemoveComponent(ComponentIds.Position);
//            _positionComponentPool.Push(component);
            return this;
        }

        void onComponentRemoved(Entity entity, int index, IComponent component) {
            OnComponentRemoved -= cachedOnComponentRemoved;
            if (index == ComponentIds.Position) {
                _positionComponentPool.Push((PositionComponent)component);
            }
        }
    }

    public partial class Matcher {
        static IMatcher _matcherPosition;

        public static IMatcher Position {
            get {
                if (_matcherPosition == null) {
                    var matcher = (Matcher)Matcher.AllOf(ComponentIds.Position);
                    matcher.componentNames = ComponentIds.componentNames;
                    _matcherPosition = matcher;
                }

                return _matcherPosition;
            }
        }
    }
}

@sschmid
Copy link
Owner Author

sschmid commented Jan 19, 2016

added onComponentRemoved

@trumpets
Copy link
Contributor

I found this article summarizing some garbage tests with events and delegates: http://jacksondunstan.com/articles/3264#comments and they confirm your findings. 104 bytes for creating a delegate and then another 208 bytes when adding that delegate to an event if it is the second+.

@sschmid
Copy link
Owner Author

sschmid commented Jan 20, 2016

I could try writing a custom event system for Entitas. Might be (or not be) more efficient

@sschmid
Copy link
Owner Author

sschmid commented Jan 20, 2016

I have an idea which works completely without events. An entity could have an array of stacks (similar to the array which holds the components). When removing a component it will get the stack at index and push the component. Done. If no stack exists, it doesn't (which means we're using entitas without the code generator)
I'll give it a try

@sschmid
Copy link
Owner Author

sschmid commented Jan 20, 2016

fyi, I started writing tests for the existing generated component extensions. After that I will implement my idea test driven.

@sschmid
Copy link
Owner Author

sschmid commented Jan 21, 2016

Ok, I found a way which works without events.
See PR #60

@sschmid
Copy link
Owner Author

sschmid commented Jan 22, 2016

@trumpets @SvDvorak
If you like, you can test the latest version. Here are the zip files
fbfc99e

I'd be interested if it solves your issues. Let me know, if it works better now

@sschmid
Copy link
Owner Author

sschmid commented Jan 24, 2016

See #60

@sschmid sschmid closed this as completed Jan 24, 2016
@SvDvorak
Copy link

SvDvorak commented Feb 2, 2016

A bit late but I checked through your solution, smart moving up the pools and creating a function to retrieve the correct one. Nicely done!

@sschmid
Copy link
Owner Author

sschmid commented Feb 2, 2016

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants