Integrated Dynamics

Integrated Dynamics

63M Downloads

Performance Optimization Discussion

Geo25rey opened this issue ยท 10 comments

commented

Issue type:

  • ๐ŸŒ Performance issue

Short description:

Continuing conversation from #1147. When Having large networks, performance degrades. I found some places where optimizations can be made using this performance profile

Potential Optimizations

The below were brought to my attention from the profile linked above since they were taking a significant amount of time to process/execute.

  1. org.cyclops.integrateddynamics.core.part.aspect.build.AspectBuilder lines 418 and 420 can be run in parallel since there doesn't seem to be any order requirements in each onUpdate method.

  2. org.cyclops.integrateddynamics.core.network.Network line 441 same recommendation as 1.

  3. org.cyclops.integrateddynamics.core.network.IngredientChannelAdapterWrapperSlotted lines 52 and 55 Setting the position state to disabled then re-enabled in the same context seems unnecessary since those changes can only happen on the main thread in series anyway.

  4. org.cyclops.integrateddynamics.core.network.IngredientChannelAdapterWrapperSlotted lines 69 and 72 same recommendation as 3.

commented

Thanks for reporting!

commented

Thanks for all the ideas @Geo25rey, I'll reply to each of them below:

org.cyclops.integrateddynamics.core.part.aspect.build.AspectBuilder lines 418 and 420 can be run in parallel since there doesn't seem to be any order requirements in each onUpdate method.

Most aspects don't have any before/after update listeners. And for the few aspects that have them, they typically only have one or two. So I suspect parallelizing their calls will probably not have any measurable impact. (there's also the open question of their thread-safety) But if you have a measurable case where things get faster with this change, I'm definitely open to it :-)

org.cyclops.integrateddynamics.core.network.Network line 441 same recommendation as 1.

See #1154

org.cyclops.integrateddynamics.core.network.IngredientChannelAdapterWrapperSlotted lines 52 and 55 Setting the position state to disabled then re-enabled in the same context seems unnecessary since those changes can only happen on the main thread in series anyway.

See #1153

org.cyclops.integrateddynamics.core.network.IngredientChannelAdapterWrapperSlotted lines 69 and 72 same recommendation as 3.

These calls are also required here, for the same reason as #1153


It looks like your profile primarily shows the getSlots call being too slow. At first glance, this looks similar to what has been reported in #1147. I already have some things in mind to resolve this, which I hope to get to soon.

If you see any other bottlenecks that I'm not aware of yet, I'm definitely open to discussing them.

commented

@rubensworks There is 1 other thing that I've noticed. The hash method for the DimPos is pretty slow. Not sure if that's a bug within your code or Apache commons, but optimizing that would certainly help.

commented

Most aspects don't have any before/after update listeners. And for the few aspects that have them, they typically only have one or two. So I suspect parallelizing their calls will probably not have any measurable impact. (there's also the open question of their thread-safety) But if you have a measurable case where things get faster with this change, I'm definitely open to it :-)

Screenshot_20220410-080440.png

commented

@Geo25rey Are you referring to hashCode or the hashing within CACHE_WORLD_KEYS? Neither are using apache commons though.

Do you have any logs where this performance impact of DimPos can be seen? (can't find it within your logs above)

commented

@Geo25rey That's something else. Those are the standard update handlers of aspects. Each aspect has exactly one update handler, which are called by the network here: https://github.com/CyclopsMC/IntegratedDynamics/blob/master-1.18/src/main/java/org/cyclops/integrateddynamics/core/network/Network.java#L409

GitHub
A Minecraft mod to take full and automated control of your appliances. - IntegratedDynamics/Network.java at master-1.18 ยท CyclopsMC/IntegratedDynamics
commented

Oh I see. Thanks @rubensworks

commented

Hey @rubensworks, sorry for the lack of progress on this issue. I don't have time during the week to work on side projects.

I do want to a little more investigation on my end, especially regarding the hashCode methods being used. (And anything else I can find)

Also, I noticed that you had an idea for an in-game performance monitor. Is there a way I can help with that?

commented

@Geo25rey Any other performance-related things you would want to take up? Or shall we close this issue here?

commented

No worries @Geo25rey, I wasn't trying to rush you :-)

I do want to a little more investigation on my end, especially regarding the hashCode methods being used. (And anything else I can find)

๐Ÿ‘Œ

Also, I noticed that you had an idea for an in-game performance monitor. Is there a way I can help with that?

Certainly! I think these issues have a high priority, but I don't think I'll have time for them in the near future: