qiao / ces.js

Component-Entity-System framework for JavaScript games.
185 stars 26 forks source link

V8 optimization #5

Open CodestarGames opened 9 years ago

CodestarGames commented 9 years ago

Is CES.js optimized for the v8 javascript engine? I noticed you were using John Resig's inheritance class for extending your objects, but in my own projects I've found that really bogs down the code optimization. I'm actually moving to using an entity component system to get away from using inheritance chains, but I see you're using them in your engine. Do you have any performance measures that you could show me?Just to ensure that If I go with CES, I won't be adding just another inheritance layer on top of my already existing one. Thanks!

josephrocca commented 9 years ago

I also can't see a reason to use the inheritance class. I haven't looked at how it works, but I wouldn't be surprised if it completely destroyed all V8's hidden class optimisation. That said, as far as I can see CES.js doesn't require you to use the inheritance class at all. This should work (not tested):

function Health(maxHealth) {
    this.name = 'health';
    this.health = this.maxHealth = maxHealth;
    this.isDead = function () {
        return this.health <= 0;
    }
    this.receiveDamage = function (damage) {
        this.health -= damage;
    }
}
//and later:
hero.addComponent(new Health(100));

And the same thing for Systems (EDIT: except you need to define this.addedToWorld() {} and removedFromWorld() {}, and also this.world = world; where world was passed to the constructor like new MySystem(world)).

I'm also wandering about optimisation in general. For example, it seems like for every System, for every world tick the following occurs:

// 1. the systems gets it's entities:
   entities = this.world.getEntities('position', 'velocity');

// 2. which is calling:
   getEntities: function () {
        return this._entities.toArray();
   }

//3. which is calling:
    toArray: function () {
        var array, node;

        array = [];
        for (node = this.head; node; node = node.next) {
            array.push(node.entity);
        }

        return array;
    }

So an array is being created by every System at every world tick? For my use case this means literally thousands of unnecessary allocations per second. Wouldn't it be better to just return the linked list (i.e. remove the .toArray() in return this._entities.toArray();)? I can't see much need for having an array rather than a linked list, and it seems like this would save a lot of unnecessary allocation. If the user isn't too worried about performance, they can call the .toArray() themselves from the System update function like: entities = this.world.getEntities('position', 'velocity').toArray();

Also, I'm not a pro with any of this stuff, but I think component pooling would be an absolute must if this library is going to be used for anything where performance is important (like thousands of components being created/destoyed every tick).

As far as I can tell, the following additions/alterations to CES.js would be required for component pooling:

  1. Keep a World.pooledComponentsByType associative array where the key is the component name and the value is an array of components ready for reuse.
  2. Alter the Entity.removeComponent(componentName) function of Entity so that the component gets pushed into the pool before the reference to the component is removed.
  3. hero.addComponent(new Position(0, 0)); --> hero.addComponent('Position', 0, 0);
  4. Keep an array (e.g. World.componentConstructors) which maps component names (e.g. 'Position') to constructors.
  5. Alter the Entity.addComponent(component) to Entity.addComponent(componentName, ...args) and inside it, check if there's any components in World.pooledComponentsByType[componentName] ready for reuse, otherwise use new World.componentConstructors[componentName](...args) to construct the component.

I'm not sure about Entity pooling. I think some profiling would be useful to see whether it's worth it. I really like the cleanness/simplicity of CES.js but just a bit of optimisation would make this library useful for larger, more performance-oriented projects.

josephrocca commented 9 years ago

Also, correct me if I'm wrong, but it looks like these two statements produce different families:

world.getEntities('Position', 'Velocity');
world.getEntities('Velocity', 'Position');

So unless I've missed something, just adding a sort in world._getFamilyId() would fix the issue:

    _getFamilyId: function(components) {
        components.sort(); // <-- just sort the names aphabetically
        return '$' + Array.prototype.join.call(components, ',');
    }
qiao commented 9 years ago

Hi @josephrocca, Thank you for your insightful suggestions.

The reason that I chose John Resig's class implementation is that it provides a very convenient API to define classes, which is quite useful for games development. Many notable JS game engines, such as ImpactJs, MelonJs, etc, all use similar mechanism to implement classes. Though I admit that it's an anti-pattern when it comes to v8 optimization, the initial objective of ces.js was more focused on developer-friendliness rather than performance. I would need to do some profiling in order to determine whether it's worth to switch to the traditional class definition.

As for the following example you provided, unfortunately it's not performant either, as every instance will have separate copies of the methods defined on itself, instead of sharing the methods by prototypes.

function Health(maxHealth) {
    this.name = 'health';
    this.health = this.maxHealth = maxHealth;
    this.isDead = function () {
        return this.health <= 0;
    }
    this.receiveDamage = function (damage) {
        this.health -= damage;
    }
}
//and later:
hero.addComponent(new Health(100));

Regarding object pooling and reusing, yes, it will be really helpful to reduce GC overhead. I'll consider implement that.

Converting the linked lists to arrays is, again, due to the aim to provide a user-friendly API, as most JS programmers would feel more comfortable to work on a native array, instead of learning the specific implementation of the linked list that ces.js provides. Nonetheless, I could define methods like forEach on the list, so that it would be more approachable to use.

Finally, the following two statements are producing different families, which is definitely a bug and thanks for pointing out.

world.getEntities('Position', 'Velocity');
world.getEntities('Velocity', 'Position');

Thank you again and happy hacking :)