Doraku / DefaultEcs

Entity Component System framework aiming for syntax and usage simplicity with maximum performance for game development.
MIT No Attribution
658 stars 62 forks source link

Entity.Set<T> Slowdown Monogame FPS #63

Closed PodeCaradox closed 4 years ago

PodeCaradox commented 4 years ago

Hello,

i have trouble to find the solutions for the following Problem: If i add this Line of Code my Fps go from 3000 to 60 in Monogame tiles[tile + j].Set<Show>(); I have 3 Systems, the first one Update my Tiles i want to draw if they are inside of my View:

protected override void Update(float elaspedTime, ref Map component)
        {
            ReadOnlySpan<Entity> tiles = mapTiles.GetEntities();
            Camera cameraData = camera.Get<Camera>();
            var tilesizex = 32;
            var tilesizey = 8;
            var XMapZeichnen = new Point((int)cameraData.VisibleArea.X / tilesizex, (int)(cameraData.VisibleArea.X + cameraData.VisibleArea.Width) / tilesizex);
            if (XMapZeichnen.X < 0) XMapZeichnen.X = 0;
            if (XMapZeichnen.Y > component.Size.X) XMapZeichnen.Y = component.Size.X;

            var YMapZeichnen = new Point((int)cameraData.VisibleArea.Y / tilesizey, (int)(cameraData.VisibleArea.Y + cameraData.VisibleArea.Height) / tilesizey);
            if (YMapZeichnen.X < 0) YMapZeichnen.X = 0;
            if (YMapZeichnen.Y > component.Size.Y) YMapZeichnen.Y = component.Size.Y;

            for (int i = YMapZeichnen.X; i < YMapZeichnen.Y; i++)
            {
                var tile = i * component.Size.X;
                for (int j = XMapZeichnen.X; j < XMapZeichnen.Y; j++)
                {
                    tiles[tile + j].Set<Show>();
                }
            }

        }

And my DrawSystem:

 [With(typeof(MapTile), typeof(MapTextureShared), typeof(Show))]
    public sealed class DrawMapSystem : AEntitySystem<float>
    {
        private readonly SpriteBatch _batch;
        private readonly TileProperty[] _tilePropertys;
        private readonly World _world;

        public DrawMapSystem(SpriteBatch batch, World world)
            : base(world)
        {
            _batch = batch;
            _tilePropertys = world.GetAllComponents<TileProperty>().ToArray();
            _world = world;

        }

        protected override void Update(float elaspedTime, in Entity entity)
        {
            ref MapTile mapTile = ref entity.Get<MapTile>();
            ref MapTextureShared mapTextureShared = ref entity.Get<MapTextureShared>();
            _batch.Draw(mapTextureShared.TextureSheet, mapTile.Position, _tilePropertys[mapTile.ID].Source, Color.White);
        }

        protected override void PreUpdate(float state)
        {

            _batch.Begin(SpriteSortMode.Deferred, BlendState.AlphaBlend, SamplerState.PointClamp, null, null, null, _world.GetAllComponents<Camera>()[0].Transform);
        }

        protected override void PostUpdate(float state)
        {

            _batch.End();
        }

    }

And one of the Start of the new Update Cycle to do the following:

[With(typeof(Show))]
    public sealed class ShowSystem : AEntitySystem<float>
    {
        public ShowSystem(World world)
          : base(world)
        {

        }

        protected override void Update(float elaspedTime, in Entity entity)
        {
            entity.Remove<Show>();
        }
    }
PodeCaradox commented 4 years ago

I fixed it to use Enable/Disable Entitys but still i don't know why the set slow down everything.

Doraku commented 4 years ago

I can see a couple of problems with what you are doing: in your ShowSystem you should not remove a component which is part of the of the entity set filter (which is probably why you had to fix something by instead enabling/disabling entities I guess?). If you want to act on components which would change the composition of the EntitySet you are currently roaming, you can either:

Based on your code I would try to change the DrawMapSystem with instead of With(typeof(Show)), WhenAdded(typeof(Show)) and WhenChanged(typeof(Show)), thus removing the need for the ShowSystem completely.

Now for it to drop to a flat 60fps, my guess is that you are actually hitting the v-sync? Is the IsFixedTimeStep of your Game edit:SynchronizeWithVerticalRetrace of your GraphicDevice instance true by any chance? (I don't know why you wouldn't get 60fps even when there is nothing to draw but that number is too specific to ignore :/)

PodeCaradox commented 4 years ago

no I used same settings like the one with 3000 fps, with enable disable entity I got arround 1000 fps and that seems ok to me. I got around 60 to 63 fps before. :/ Also is the Problem from before maybe because I splitted the update and the draw systems separately and update them in the Update/Draw method from Monogame?

        protected override void Update(GameTime gameTime)
        {
            if (GamePad.GetState(PlayerIndex.One).Buttons.Back == ButtonState.Pressed || Keyboard.GetState().IsKeyDown(Keys.Escape))
                Exit();

            _updateSystem.Update((float)gameTime.ElapsedGameTime.TotalMilliseconds);

            base.Update(gameTime);
        }

        protected override void Draw(GameTime gameTime)
        {
            GraphicsDevice.Clear(Color.CornflowerBlue);

            _drawSystem.Update((float)gameTime.ElapsedGameTime.TotalMilliseconds);

            base.Draw(gameTime);
        }

I tested your 3rd suggestion, but it will also give me the old tiles i set the times before in the Update Method. I only need the actual from the actual Iteration.

public sealed class DrawMapSystem : AEntitySystem<float>
    {
        private readonly SpriteBatch _batch;
        private readonly TileProperty[] _tilePropertys;
        private readonly World _world;

        public DrawMapSystem(SpriteBatch batch, World world)
            : base(world.GetEntities().With<MapTile>().With<TextureShared>().WhenAdded<IsVisible>().WhenChanged<IsVisible>().Build())
        {

            _batch = batch;
            _tilePropertys = world.GetAllComponents<TileProperty>().ToArray();
            _world = world;

        }

        protected override void Update(float elaspedTime, in Entity entity)
        {
            ref MapTile mapTile = ref entity.Get<MapTile>();
            ref TextureShared mapTextureShared = ref entity.Get<TextureShared>();
            _batch.Draw(mapTextureShared.TextureSheet, mapTile.Position + mapTile.Offset, _tilePropertys[mapTile.ID].Source, Color.White);
        }

        protected override void PreUpdate(float state)
        {

            _batch.Begin(SpriteSortMode.Deferred, BlendState.AlphaBlend, SamplerState.PointClamp, null, null, null, _world.GetAllComponents<Camera>()[0].Transform);
        }

        protected override void PostUpdate(float state)
        {

            _batch.End();
        }

    }

Thanks for your help and the open Project.

Doraku commented 4 years ago

You removed the base.PostUpdate(state) in your override, that's why you still have the old tiles (in the case of a reactive EntitySet it clear it of processed Entity, I should probably do something to make it clearer!) Edit: and I should have told you that in my first response already >_< I missed it

But now that I think of it, because you have splitted the update and draw systems (which is not bad, I am doing the same on my project), since the DrawMapSystem will clear its entities only when it is called, if you have multiple calls to monogame Update before the Draw (because of IsFixedUpdate, in case you want to change it later, and a slow down occure making the update catchup the lost time), you might draw tiles of previous frame :/ Hopefully this should not happen to much and worst case you do not over draw too many tiles for one frame.

PodeCaradox commented 4 years ago

Ah thanks Doraku, now it works.

Doraku commented 4 years ago

Ah great, did you got back to an acceptable framerate? See the edit of my previous post for some futur problems just in case ^^"

PodeCaradox commented 4 years ago

Yea i do get around 1.6k FPS. Yea old tiles get Drawn can happen, maybe i find also another way to do it better than the approach for now.

One question if i disable entity's, how can i get them back from the Entity World?

Doraku commented 4 years ago

Currently, the only way to get back disabled entities is with the [World.GetAllEntities](https://github.com/Doraku/DefaultEcs/blob/master/documentation/api/DefaultEcs-World-GetAllEntities().md) method which give you all entities but I don't recommand it. I do have a local branch where I added a way to get disabled entities in EntitySet but I am conflicted about it as it bloates EntitySet code. For my game, I have a custom queue of predefined entities disabled ready to be used for my particles (most components already set to speed up things, as changing value cost less), basically a pool. If you disable an entity you get it out of EntitySet, so you have to handle it yourself to enable it back.

This works for particles but for other case it can be strange, like loading all your enemies in advance, disabled, and wanting to enable them as needed. This is a case I will soon face so I will probably revisit this old branch. Most features of this project come from my direct needs and some from resquest, discussion or suggestion of other kind people here (like the react EntitySet ^^) so don't hesitate ;)

PodeCaradox commented 4 years ago

Alright thanks for the answer, i thought i did overlooked it. So i can handle the Disabled Entity's by myself, that's fine.

Maybe you can change the Build() method to Build(GetDisabledEntitys: true) (standard false) to know if i want a EntitySet with only disabled Entity's or Enabled ones. :)

But great work, your defaultEcs is great with good Documentation. :)

Also a suggestion: For the runner System is it possible to set a count for how many Entity's should be processed by one thread and if there are more to split it into more threads?

PodeCaradox commented 4 years ago

More investigation for better Performance:

I use now the disable/enable for the Component, that give me much better Performance on a big Map with 5000 * 5000 Tiles. Somehow the Entity.Set have a Problem when i go to different position in the Map(Fps go from 800 to 200), if i use instead the disable/enable it stays at 800.

Preview(even if i change shared texture no fps drops :)): Preview

Doraku commented 4 years ago

I created 2 issues to track your suggestion

64 on my branch it is a World.GetDisabledEntities method instead of overcharging the Build method but same result :)

65 I had this idea already but the limit would be on AEntitySystem (and AComponentSystem) so you can reuse the same SystemRunner and each system having its own minimal count (still need to work on #61 first though!)

So... am I reading that right that you have 25 000 000 entities just for tiles x)?! While I am quite happy that you are stress testing my framework like so I wonder if this is the right way haha.

Thanks for the kind words I am quite interested to see where you will go from here :)

PodeCaradox commented 4 years ago

The big map was just to test if it also work with very big Map and it worked(also ram usage was not so high) . I tested it because I wanted to see if there is a problem with the entity systems to drop frames but with disable/enable all work just fine.

Thanks for adding my ideas as issues, when I'm find new issues I will add them in the issues section :)

If you want i can update you to my Project i am doing :) I make a isometric strategy Game: For now i add height to the Map and add a Editor ;) grafik

Doraku commented 4 years ago

Sure, I always wonder if my library feel easy to use to everyone or just to me x) and how it behaves for different type of games, mine is a simple shooter (and I am taking forever to work on a fully dynamic editor handling any type of component to create World state as level).

PodeCaradox commented 4 years ago

Hey Doraku,

thanks for the fast Update with the Disabled Entities.

I got a Problem, is there a fast way to enable disable a component or maybe a Entity from about 1000 or more Entities at a Time? Maybe you have a good idea for my Problem: I have this struct:

public struct IsVisible
    {
    }

And in each Update i run this system:

    [With(typeof(IsVisible))]
    public sealed class IsVisibleSystem : AEntitySystem<float>
    {
        public IsVisibleSystem(World world)
          : base(world)
        {

        }

        protected override void Update(float elaspedTime, in Entity entity)
        {
            entity.Disable<IsVisible>();
        }
    }

And than calculate the visible area to only enable my Entities in the view:

  for (int i = YMapZeichnen.X; i < YMapZeichnen.Y; i++)
            {
                var tile = i * component.Size.X;
                for (int j = XMapZeichnen.X; j < XMapZeichnen.Y; j++)
                {

                    if(_tiles[tile + j].Get<MapTile>().IsVisible)
                        _tiles[tile + j].Enable<IsVisible>();
                }
            }

            _quadrantSystem.EnableEntitys(new RectangleF(new PointF(cameraData.VisibleArea.X, cameraData.VisibleArea.Y),new SizeF(cameraData.VisibleArea.X + cameraData.VisibleArea.Width, cameraData.VisibleArea.Y + cameraData.VisibleArea.Height)));

grafik But with only disable the Component i lose about 300 Fps :/ ,and with the enable on my Soldiers Entities i lost also around half of it. Maybe you know how i can do it faster, or have a good idea.

Maybe for the disable, i can have a Method to disable all: components[ComponentManager.Flag] = false

I know i stress Test your EntitySystem very hard with so many Entities :D

Doraku commented 4 years ago

Hopefully I don't make too many bad assumptions about what you are doing. Your _tiles is a flattened array of all your tiles, row by row, both MapZeichnens give you the indexes of the tiles to be shown for the frame, each tile having its own entity to represent it.

I could add a mass disable for a given component type but I am not sure this is a good practice, while there is some speed to gain in clearing EntitySet with such a method, I would have to modify the components of each entity, even the one without the flag.

Naively each frame a lot of tile from the previous frame should still be visible so clearing the whole screen seems counterproductive. Maybe you should keep the screen coordinate of the frame n-1, and just activate/deactivate the tiles that actually change? Since it is just 2 rectangle intersecting there should be a simple way to get the changing parts with the way your tiles are arranged.

An other way coming to mind would be to only "create" entities for visible tiles. Let's say you have a pool of entities, each frame you enumerate all the visible tiles and set the entity's components to represent it (since you are reusing the same entities, they actually shouldn't change composition, just the values, which is much faster, the EntitySet would not change). disabling and enabling entities in your pool in case the number of tile shown change (it shouldn't have much variance I suppose). With this method you would save a lot of space (entities) too I believe!

As for the soldiers... obviously those need to keep their own entities so they can still move around the world/attack/whatever even when they are not visible. You could speed up the process of showing them or not by using a quadtree to separate your map in smaller zones. That way instead of checking every units, you only check the ones in zones shown on the screen.

PodeCaradox commented 4 years ago

For the Soldiers i am using already a Quadtree, but i am testing performance for 10000 Soldiers at once at the Screen, but here i can do some improvement, for example cache some things from the frame before to look which Entities got changed and only these need to be checked again, so i don't need to disable all visibility and enable again).

For the Tiles i think your idea with Pooling could be nice, i will test it with that and than only change the the Property's so it just works without visible. The idea is create because if i want i can also Stream the Terrain if i want. I implemented Tiles, its working now without any Fps loses, i did it like the Code below shows and it work perfectly :)

  for (int i = YTilesVisible.X; i < YTilesVisible.Y; i++)
            {
                var tile = i * component.Size.X;
                for (int j = XTilesVisible.X; j < XTilesVisible.Y; j++)
                {

                    ref MapTile tileData = ref _displayedTiles[k].Get<MapTile>();
                    tileData = _tiles[tile + j];
                  k++;
                }
            }
Doraku commented 4 years ago

Great to know :) any operation making composition change on entity (and thus changing EntitySet content) will always cost more than just changing component values because of the way every thing is implemented. I took this design choice because to me, your entities shouldn't change much once you have loaded your world. Obviously you can still have new elements coming in but not in the scale of your previous code (although it still seemed to hold the charge, nice x)!). I hope this choice will not bite me in the futur ^^"

10000, you are making a total war or something haha? Yeah changing the visibility every frame will cost. Hum just throwing it here but what about using the same strategy as for tile? Unit would basically use 2 entities, one for game logic, and if visible, one taken from an array of initialized entities to be displayed?

PodeCaradox commented 4 years ago

Its a little bit like the old Stronghold Games but with another touch to it.

Hm how can i use the same Method for entities? The array will change how many are visible all the Time maybe there are 1000 on the screen or only 5. So i will do use garbage or disable entities :/ .Or what exactly you mean by using 2 entities?

Doraku commented 4 years ago
class UnitVisibilitySystem : ISystem<whatever>
{
    private readonly Entity[] _visibleUnits;
    private readonly QuadTree _units;

    private int _visibleCount;

    private static Entity CreateVisibleEntity(World world)
    {
        Entity entity = world.CreateEntity();
        entity.Set<IsVisible>();
        entity.Set<Position>();
        // set whatever is needed to display, and only display, this entity should not fall into a game logic system EntitySet

        return entity;
    }

    public UnitVisibilitySystem(World world, QuadTree units)
    {
        _units = units; // or get it from an entity in the world? not sure how you handle it
        // whatever you think is the max number of unit which can be displayed on screen, change it to a smarter collection to build on demand entity if needed
        _visibleUnits = Enumerable.Repeat(0, 1000).Select(_ => CreateVisibleEntity(world)).ToArray();
        _visibleCount = 0;
    }

    public void Update(whatever)
    {
        int newCount = -1;
        foreach (Entity unit in _units) // probably something more complicated to get the visible units :p
        {
            Entity visibleUnit = _visibleUnits[++newCount];
            visibleUnit.Enable<IsVisible>();
            visibleUnit.SetSameAs<Position>(unit); // might actually be faster to do visibleUnit.Get<Position>() = unit.Get<Position>() depends on the size of your components
            // set what is needed to deplay the unit
        }

        for (int i = newCount + 1; i < _visibleCount; ++i)
        {
            _visibleUnits[i].Disable<IsVisible>();
        }
        _visibleCount = newCount;
    }
}

Basically, what you want to avoid is a huge amount of entity change in EntitySet. Your unit Entity contains only game logic components, while you use the _visibleEntities to actually set what you really want to display. Between 2 frames you should not have such a big difference of displayed unit, so the cost of enable/disable is reduced a lot (except if you click on the mini map to change the displayed zone but it only last one frame).

But what you proposed to only check for changed units would probably be less cumbersome.

PodeCaradox commented 4 years ago

Hey thanks again for the advice with the Soldiers, i have now all working and get much more Fps that i expected. I can have around 200 000 Soldiers on Screen(off screen much much more :D) and still have many Fps. Later it will be less because of path finding and other things, but i think i can do the other system with Multi threading for more Cpu Usage. Here a GIF, i uploaded it somewhere else because it was to big(max 10mb images on github): https://gifyu.com/image/vYjn

Doraku commented 4 years ago

I guess you are done with the stress testing for now haha

PodeCaradox commented 4 years ago

Hey Doraku, is there a way to have one entity that holds all components i need(for example a Soldier) and than initial other (Soldiers)Entities with that and they automatically have all Components from the first Entity? Or is there a way with a foor loop to go over a entities Components?

Doraku commented 4 years ago

Yes to both! Entity.CopyTo will duplicate your entity in the passed world instance (know that it will duplicate each component value in the copy) Entity.ReadAllComponents to enumarate all the entity component. You need to provide your own IComponentReader implementation though (this is done to avoid boxing and copy of struct component)

You are not the first one to ask that, I should probably add this question to the faq.

PodeCaradox commented 4 years ago

Yeah i overlooked that, sorry for that. But thanks for the Response :) Also do the Entity.CopyTo Copy SetSameAs(Entity) things as Same reference?

Doraku commented 4 years ago

Ah sorry I didn't see your edit. You probably have found out for yourself by now but no, CopyTo use Set internally, this is because there is no main owner of a component shared with SetSameAs (not technically true, a reference entity act as the main owner internally but it can change ownership if this entity is disposed/remove this component so we can't really know if we are supposed to do a Set or a SetSameAs anymore and I chose the poor lazy solution of only doing Set).

PodeCaradox commented 4 years ago

Thanks for the explanation, but than do SetSameAs override a Set Component, so i can do after copy it add it by myself?

Doraku commented 4 years ago

Yes, SetSameAs after a Set will replace the existing value with the shared one. Doing a SetSameAs after an other SetSameAs will not change the previous shared value. But be careful, doing a Set after a SetSameAs will change the shared value of every entities sharing it, it will not give the entity its own value, you would need to first Remove the component on the entity.

Wow by typing that I realize this might not be what users expect (the Set not giving the entity its own value), I wonder if I should change it or say a word in the readme.

PodeCaradox commented 4 years ago

Hmm if you changed it i cant change the shared value(maybe a method SetSharedValue to change it), but the user would expect to change only the value with set, so normal Set behavois only on the entity and removes the SharedCompnent from it, and a SetSharedValue Method to change the Shared Value? I don't need it often but sometimes i need to change sharedValues for all Entitys. For Example it start to snow on my Map so i only change the SharedTexture and all Tiles look Snowy :).

In my Game i use it to share Texture Sheets for my Objects, for more fps on the GPU, so i don't need to change the buffer so often = less sending Textures to GPU.

Doraku commented 4 years ago

You could always change the shared value by using entity.Get<Component>() = newValue; but this will not trigger an update component message (in case you are using WhenChanged<> directive for your EntitySet). I will probably keep it as is for now, SetSameAs is explicit but with Set you probably just want to change the value, whether this value only belong to your entity or if it is shared should not make a difference.

PodeCaradox commented 4 years ago

@Doraku I still think your DefaultEcs is very nice, i get totally all to work with it. I have here a update Gif from my Project for you :3 Preview2

And also i can handle so much units xD grafik

Doraku commented 4 years ago

Wow, that waterfall effect is so simple but looks so good! You are miles ahead of my game, I should stop adding features to DefaultEcs and work properly on it x) I wish you all the best to see the end of your project and release it :) Then would you be ok for me to add a link to it on the readme as a proper game using this framework? It would be a much better showcase than my poor samples! Did you encounter any strange quirk when using DefaultEcs to make this? Something counter-intuitive and behaving not like what you expected? I am too biased as the maker so any feedback would be greatly appreciated!

PodeCaradox commented 4 years ago

Hm the only thing was the slowdown from the remove entity component with 10k Units, But with the disable Component, it was much smother. All other things are nice and with the updates its easier to understand 👍

Yeah you can add a link as a showcase, if i finished the Project. :)

PodeCaradox commented 4 years ago

@Doraku is it maybe faster to set not a component and remove it every update, but instead use a shared Component just as a Tag. For example remove every Update the IsInCameraBounds and set it as a shared Component for the new Tiles shown? I did test it with a Map Brush and here it worked for me, maybe i try the Method with the shared Component if this also works, without losing to much performance.

Doraku commented 4 years ago

If your type is a memberless struct (ex: public struct IsInCameraBounds { }) it will automatically be handled as a unique shared component. While much faster than setting a normal component, it is still is more expensive than just enabling/disabling components. This is a break down of what happens: For Set/Remove:

For Enable/Disable:

I do have the project of bringing the performance of flag component Set/Remove on par with the Enable/Disable case (#69), I definitely should look more seriously into that.

I believe the best performance you could get would be to have a quad-tree view of your entities (I think your already have something like this anyway but you can achieve it easily with a [MultiMap](https://github.com/Doraku/DefaultEcs/blob/master/documentation/api/DefaultEcs-EntityRuleBuilder-AsMultiMap-TKey-().md) now, see this sample for an example), each update you check the concatenation of the entities from the previous screen position and the new screen position (most of the quad-tree zones of the two frames should be in common) to Set/Remove or Enable/Disable your IsInCameraBounds component.

PodeCaradox commented 4 years ago

Ok thanks, i didn't looked so much in your Code, i hoped it maybe bypass the Frame Drops :)

PodeCaradox commented 3 years ago

@Doraku Here a Game for a GameJam i made this weekend with your DefaultEcs and Monogame, its on some places ugly but maybe as Demo or something you can need. github: https://github.com/PodeCaradox/HellowIInJam

Doraku commented 3 years ago

Hey congrats on finishing the jam! Would love to look around your code but I think your repo is private.

To simplify usage, since the new release of Visual Studio last week I have been working on a source generator analyzer to be much more expressive and safe, basically instead of

        private sealed class DefaultEcsSystem : AEntitySystem<float>
        {
            public DefaultEcsSystem(DefaultWorld world, IParallelRunner runner)
                : base(world.GetEntities().With<DefaultSpeed>().With<DefaultPosition>().AsSet(), runner)
            { }

            public DefaultEcsSystem(DefaultWorld world)
                : this(world, null)
            { }

            protected unsafe override void Update(float state, in Entity entities)
            {
                DefaultSpeed speed = entity.Get<DefaultSpeed>();
                ref DefaultPosition position = ref entity.Get<DefaultPosition>();

                position.X += speed.X * state;
                position.Y += speed.Y * state;
            }
        }

You would be able to just declare

        private sealed partial class DefaultEcsGeneratorSystem : AEntitySystem<float>
        {
            [Update]
            private static void Update(float state, DefaultSpeed speed, ref DefaultPosition position)
            {
                position.X += speed.X * state;
                position.Y += speed.Y * state;
            }
        }

and the constructors and update override would be automatically generated for you with the correct EntitySet composition. (I will never finish a game haha)

PodeCaradox commented 3 years ago

Nice that will help alot for faster writing, yeah i just changed it to public xD