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

EntityReference - is convoluted, complex and inefficient. #7195

Open
Maksims opened this issue Dec 13, 2024 · 1 comment
Open

EntityReference - is convoluted, complex and inefficient. #7195

Maksims opened this issue Dec 13, 2024 · 1 comment
Labels
performance Relating to load times or frame rate

Comments

@Maksims
Copy link
Collaborator

Maksims commented Dec 13, 2024

Various Components use EntityReference to somewhat simplify events management, but its convoluted implementation, has a lot of inefficient and unnecessary mechanics, like subscribing to a large EventHandlers, and removing such subscriptions often.
So when such Components are added or removed, there is a significant amount of time spent in EntityReference.

Having 100 lines of comments describing what it does, and then having a complex implementation that is still hard to understand - makes it even worse.

The events management is inefficient as there was no EventHandle at the time, so events removals from ComponentSystems - is slow. By utilizing EventHandle.off some speedup can be achieved. It also subscribes to a large EventHandlers like ComponentSystem only to check in callback if it relates to our component - this of course leads to a very large callback lists - which just slows everything down on fires.

A simple example is RenderComponent.rootBone - which is just a reference to another entity. And can be a simple property, and does not need to be a complex system behind it. This PR showcases this. And leads to ~87% speedup improvement for component removal.

Other Components need to do a similar job by simplifying properties, and implementing their logic directly inside of a component, instead of using convoluted EntityReference.

This will lead to massive speedups and move logic implementation to its relevant place - the Component itself.

@Maksims
Copy link
Collaborator Author

Maksims commented Dec 17, 2024

Here is described problem in simplified code example:

class ComponentSystem extends EventHandler {

}

class Component {
    constructor() {
        // in every EntityReference
        this.system.on('add', this.onAdd, this);
        this.system.on('beforeremove', this.onRemove, this);
        //
    }

    // { in an EntityReference:
    onAdd(entity) {
        if (this.entity === entity) {
            // do something
        }
    }
    //

    // { in an EntityReference:
    onRemove(entity) {
        if (this.entity === entity) {
            this.system.off('add', this.onAdd, this);
            this.system.off('beforeremove', this.onRemove, this);
        }
    }
    //
}

Each EntityReference would subscribe to whole system, and then check if this.entity === entity. So when we create a new component, we subscribe to whole system again, and when we remove component, we unsubscribe, so if there is a lot of these component, or even components subscribing to the same system (like Element, Button, Scrollbar, ScrollView, and possibly others), this leads to an enormous list of callbacks which do if (this.entity === entity) { to check if it relates to them.

So the cost of creating and removeing of a new component grows exponentially with size of a scene. And the size does not need to be crazy to start getting lag spikes on every hierarchy modification (adding, removing, reparenting).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Relating to load times or frame rate
Projects
None yet
Development

No branches or pull requests

2 participants