Proposal for component lifecycle methods

[geist-2501]

Hi all!

I've been tinkering with Fleks and, while the component hooks are lovely and work perfectly, I wondered why there isn't lifecycle methods on components instead.

My specific use-case is that my components can raise events in order to talk to the UI. To do this they need an event manager which I inject - however, components don't have the whole Fleks context thing so I can't inject directly. So instead, I have to initialise it via a hook (which do have a Fleks context), which works great, but took me a while to think of 🗿.

Wouldn't lifecycle methods be more developer friendly? For instance;

class ToolbarComponent : Component<ToolbarComponent> {

    private lateinit var eventManager: EventManager

    override fun World.onAddComponent() {
        eventManager = inject()
    }

    // Instead of;

    companion object : ComponentType<ToolbarComponent>() {
        val onAddComponent: ComponentHook<ToolbarComponent> = { entity, toolbarComponent ->
            toolbarComponent.eventManager = inject()
        }
    }
}

I've forked this repo and got the lifecycle methods working, but I'd be interested to know peoples opinions on whether this is actually a better approach :)

Also Fleks is awesome! Very happy that I made the switch from Ashley halfway through my project.

[Quillraven]

My reason for it was that I thought you don't need these methods for every single component AND performance reasons.

What would be interesting for me is how much slower it gets when we add onAdd/onRemove as default functions to a component and call it in the specific ComponentHolder functions where the hooks are currently called.

There is a benchmark sourceSet with a manual.kt file in it. You can run it before and after your changes for a quick comparison. Run it a few times and take the average values.

This is what I usually do to test such changes. The gradle benchmarkJvm task is the real one but takes veeeery long, that's why I don't recommend it for quick testing 😉

Also, ignore the result in the comments of manual.kt file. You will most likely have different numbers. Those are the ones from my PC which was also used for the benchmark table in the README.md.

My laptop e.g. has completely different values and you will most likely also have different ones. That's why you should compare it with your own numbers and run it before and after your changes.

[geist-2501]

Ah! I never considered that it could have a performance impact - I figured it would be the same as with the hooks.

I did a spot of research, and even though the default implementation of the lifecycle methods would just be fun World.onAddComponent() = Unit, the function call would still exist in the bytecode. It is up to the JVM implementation to optimise the call away, which will vary from user to user :( - and I have absolutely not a clue for Kotlin Native / Multiplatform.

Performed the benchmarks - from what I can see, there isn't much difference 🎉. I ran 3 batches of benchmarks at different times of the day, and each benchmark ran 10 times instead 3 times to smooth out variance.

Before;

  ADD_REMOVE:
Artemis: max(20)    min(2)  avg(6.5) 
Artemis: max(22)    min(2)  avg(6.5)
Artemis: max(20)    min(1)  avg(6.4)  
Fleks:   max(16)    min(2)  avg(5.7) 
Fleks:   max(16)    min(3)  avg(5.9) 
Fleks:   max(16)    min(3)  avg(5.7)
  SIMPLE:
Artemis: max(26)    min(14)  avg(18.2)
Artemis: max(19)    min(15)  avg(16.3) 
Artemis: max(16)    min(13)  avg(14.2)  
Fleks:   max(18)    min(15)  avg(16.6)  
Fleks:   max(26)    min(24)  avg(25.3) 
Fleks:   max(23)    min(16)  avg(18.4) 
  COMPLEX:
Artemis: max(662)    min(588)  avg(610.0) 
Artemis: max(698)    min(585)  avg(615.4) 
Artemis: max(653)    min(574)  avg(611.4)
Fleks:   max(973)    min(784)  avg(862.3)
Fleks:   max(988)    min(761)  avg(857.9)
Fleks:   max(1043)    min(838)  avg(947.2) 

After;

  ADD_REMOVE:
Artemis: max(22)    min(1)  avg(7.2)  
Artemis: max(21)    min(2)  avg(6.9)  
Artemis: max(20)    min(1)  avg(6.4)  
Fleks:   max(16)    min(2)  avg(5.6)
Fleks:   max(15)    min(2)  avg(5.5)
Fleks:   max(15)    min(2)  avg(5.6)
  SIMPLE:
Artemis: max(25)    min(15)  avg(17.7) 
Artemis: max(19)    min(15)  avg(17.8) 
Artemis: max(20)    min(13)  avg(15.2) 
Fleks:   max(33)    min(27)  avg(29.9) 
Fleks:   max(19)    min(17)  avg(18.2) 
Fleks:   max(33)    min(27)  avg(30.4) 
  COMPLEX:
Artemis: max(639)    min(582)  avg(606.2) 
Artemis: max(681)    min(597)  avg(632.4) 
Artemis: max(685)    min(566)  avg(607.3) 
Fleks:   max(952)    min(750)  avg(860.4)  
Fleks:   max(948)    min(744)  avg(843.4)  
Fleks:   max(943)    min(751)  avg(845.3)  

[Quillraven]

Looks indeed very similar. I could imagine it having an impact on Android. From what I know there is a limited amount of functions you can put on the stack. But theoretically it will be the same if you'd use a hook for each component. Otherwise, the current version is more optimized for that particular backend.

Out of interest please run the BenchmarkJvm gradle task with and without the changes and share your result.

If it is still nearly the same then I don't have a problem to change it if people prefer that.

[JonoAugustine]

If it is still nearly the same then I don't have a problem to change it if people prefer that.

It would definitely be simpler for new users to pick up on and make the code a little cleaner to write. This would remove the need to add hooks to the world definition as well, if I'm understanding it correctly.

[geist-2501]

This would remove the need to add hooks to the world definition

Maybe, but technically the hooks can be defined in any scope and take any variable as a closure (not necessarily good or bad in my opinion). Lifecycle methods would be restricted to the variable scope of the component.

I.e.;

val foo = Foo()
engine = world {
  components {
    onAdd(Bar, { entity, barComponent ->
      barComponent.thing = foo
    })
  }
}

Although one could use injectables to access foo from a component lifecycle method, so this might not be an issue.

[geist-2501]

And the results from jvmBenchmark are in! Looks like there is little difference, although I'm not sure how thin the margins are supposed to be.

Before;

ArtemisBenchmark.addRemove  thrpt    5  913.614 ± 399.140  ops/s
ArtemisBenchmark.complex    thrpt    5    1.750 ±   0.045  ops/s
ArtemisBenchmark.simple     thrpt    5   62.403 ±  49.924  ops/s
AshleyBenchmark.addRemove   thrpt    5  252.327 ±   6.652  ops/s
AshleyBenchmark.complex     thrpt    5    0.073 ±   0.011  ops/s
AshleyBenchmark.simple      thrpt    5   18.467 ±  15.777  ops/s
FleksBenchmark.addRemove    thrpt    5  527.525 ±   6.774  ops/s
FleksBenchmark.complex      thrpt    5    1.246 ±   0.043  ops/s
FleksBenchmark.simple       thrpt    5   48.811 ±  59.562  ops/s

After;

ArtemisBenchmark.addRemove  thrpt    5  932.959 ± 333.593  ops/s
ArtemisBenchmark.complex    thrpt    5    1.798 ±   0.040  ops/s
ArtemisBenchmark.simple     thrpt    5   63.018 ±  48.826  ops/s
AshleyBenchmark.addRemove   thrpt    5  260.227 ±   6.953  ops/s
AshleyBenchmark.complex     thrpt    5    0.074 ±   0.008  ops/s
AshleyBenchmark.simple      thrpt    5   18.042 ±  15.172  ops/s
FleksBenchmark.addRemove    thrpt    5  527.831 ±  13.872  ops/s
FleksBenchmark.complex      thrpt    5    1.280 ±   0.027  ops/s
FleksBenchmark.simple       thrpt    5   45.945 ±  63.629  ops/s

[Quillraven]

Interesting that Artemis simple benchmark performs way faster on your machine. For me on both PC and LAPTOP it is always slightly slower.

Anyway, those benchmarks are imo "extreme" scenarios and the little slowdown in the simple benchmark is not that big of a deal I'd say.

As you mentioned, the component hooks are then no longer necessary and can be removed (entity hooks are still needed though) and I agree, it makes it easier to define the add/remove component logic. Since it runs in the context of the world, you also have access to everything through injectables.

Do you want to create a PR for it? I can review it and it can be part of 2.4 which will be released in the near future.

[geist-2501]

Yeah I'll hopefully make a PR within the next few days!

[Quillraven]

@geist-2501: I am currently migrating my example projects to 2.4-Snapshot and I wonder, if it is possible to pass the component as a second argument to the lifecycle methods. The reason is, that I don't like the annotated this variables a lot like here:

override fun com.github.quillraven.fleks.World.onAddComponent(entity: Entity) {
    this@PhysicComponent.body.userData = entity
}

override fun com.github.quillraven.fleks.World.onRemoveComponent(entity: Entity) {
    val body = this@PhysicComponent.body
    body.world.destroyBody(body)
    body.userData = null
}

I want to just name it physic or physicCmp. Do you think this is feasible?

edit: oh, I just notice you can simply remove the entire this part because it runs in the context of the component as well. I think this is fine then - nevermind!

[geist-2501]

Yup - I was just about to to say that! :-)

As a side note - I've updated a fork of the wiki to contain the new lifecycle method stuff - would you like me to open a PR for it or will you perform the wiki update yourself? It seems like contributing to a project's wiki is a little bit janky.

[Quillraven]

I plan to update the wiki after 2.4 is live but if you already have something then please share it! Maybe just copy&paste the markdown into this issue then I can take it over.

[geist-2501]

Updated the wiki locally and I can provide it as a patch file - just like in the olden days before PRs 😛

You can use git apply <.patch file> to apply a patch - hopefully that should work!

wiki.patch

[Quillraven]

Thank you! I cannot apply the patch because it seems like it tries to find a Component.md/Entity.md/... file which does not exist (?).

Anyway, I will then manually take over your changes and I will keep the ComponentHook info for history reasons, if people are using a version before 2.4. Maybe on a separate wiki page.