Kelp is quite expensive #3395

New Issue

ancientmarinerdev · 2023-02-04T03:12:47+01:00

ancientmarinerdev commented

2023-02-04 03:12:47 +01:00

MineClone2 version: master

What happened?

When flying over ocean profiling, kelp code is very expensive.

While mapgen is complex, heavy and does a lot of work (19.15%), mcl_ocean/kelp is hogging some resources on 6.6%. See attached profiling visualisation.

kelp.surface_on_timer(pos) is the function in question. Looks like each kelp on place has a 0.2s step timer set to process growing etc.

Alongside all our surface floating kelp, this is not idea and does need a rework. Considering it's also the best way of creating water source blocks for bubble columns, kelp really needs some love.

What should happen:

Profile on load and just sitting over water. Investigate and see if this can be reworked. Could be part of a larger kelp rework.

MineClone2 version: master ### What happened? When flying over ocean profiling, kelp code is very expensive. While mapgen is complex, heavy and does a lot of work (19.15%), mcl_ocean/kelp is hogging some resources on 6.6%. See attached profiling visualisation. kelp.surface_on_timer(pos) is the function in question. Looks like each kelp on place has a 0.2s step timer set to process growing etc. Alongside all our surface floating kelp, this is not idea and does need a rework. Considering it's also the best way of creating water source blocks for bubble columns, kelp really needs some love. ### What should happen: Profile on load and just sitting over water. Investigate and see if this can be reworked. Could be part of a larger kelp rework.

villager_sea.svg

98 KiB

👍 1

ancientmarinerdev added this to the 3 - Near future milestone 2023-02-04 03:12:47 +01:00

ancientmarinerdev added the

performance

#P3: elevated

labels 2023-02-04 03:12:47 +01:00

Michieal commented

2023-02-04 09:52:26 +01:00

Kelp shouldn't use on_timer for growth. That's insane. When I tested using node timers for bamboo, it broke often (was really unreliable) and was not performant at all.

Plant growth should be in an ABM, and it should be done intelligently. While I have seen kelp floating items in Minecraft in my expeditions, it's not been as many as what MCL2 has. Node timers have their uses, but there's a LOT of maintenance to have them and making sure that they shut off properly. And, as I said, they aren't guaranteed to run. Making a performant abm for kelp should be a (as noted) priority. And, that .2 second timer is most likely why we have a ton of kelp, and why it lags and causes holy flying kelp issues.

Kelp shouldn't use on_timer for growth. That's insane. When I tested using node timers for bamboo, it broke often (was really unreliable) and was not performant at all. Plant growth should be in an ABM, and it should be done intelligently. While I have seen kelp floating items in Minecraft in my expeditions, it's not been as many as what MCL2 has. Node timers have their uses, but there's a LOT of maintenance to have them and making sure that they shut off properly. And, as I said, they aren't guaranteed to run. Making a performant abm for kelp should be a (as noted) priority. And, that .2 second timer is most likely why we have a ton of kelp, and why it lags and causes holy flying kelp issues.

👍 1

Michieal commented

2023-02-04 10:05:30 +01:00

#3061 (kelp item entities caused during mapgen) and #3071 & #1565
#3104

Look at the Nodestep stuff in this issue: #2564

#3061 (kelp item entities caused during mapgen) and #3071 & #1565 #3104 Look at the Nodestep stuff in this issue: #2564

ancientmarinerdev referenced this issue

2023-02-09 00:11:49 +01:00

Change kelp to use ABMs to solve performance issues #3417

ancientmarinerdev added

#P2: HIGH

and removed

#P3: elevated

labels 2023-02-10 01:04:27 +01:00

ancientmarinerdev removed the

#P2: HIGH

label 2023-02-15 17:49:13 +01:00

ancientmarinerdev commented

2023-02-15 17:51:26 +01:00

"The biggest expense remaining is the ABM that removes unsubmerged. This makes farms work, and really needs improvement functionality wise. The only reason I can see for the kelp farm working is because of the piston removing the water source block and the kelp being unsubmerged and dropping.

There are further issues such as broken kelp in flowing should replace it with a source block. This is a way to create a bubble column. I will create a ticket to pull all this information together, but in terms of hammering performance, this is greatly improved and more maintainable."

I have lowered the priority of this part as it could need some piston work and more kelp work but it's no longer massively impacting performance as it was before.

"The biggest expense remaining is the ABM that removes unsubmerged. This makes farms work, and really needs improvement functionality wise. The only reason I can see for the kelp farm working is because of the piston removing the water source block and the kelp being unsubmerged and dropping. There are further issues such as broken kelp in flowing should replace it with a source block. This is a way to create a bubble column. I will create a ticket to pull all this information together, but in terms of hammering performance, this is greatly improved and more maintainable." I have lowered the priority of this part as it could need some piston work and more kelp work but it's no longer massively impacting performance as it was before.

ancientmarinerdev added this to the Essential farms project 2023-02-15 17:52:05 +01:00

ancientmarinerdev removed this from the Essential farms project 2023-03-21 14:20:33 +01:00

ancientmarinerdev added the

#P4 priority: medium

label 2023-04-17 00:00:12 +02:00

ancientmarinerdev added the

#Review

label 2023-05-09 16:31:13 +02:00

ancientmarinerdev removed this from the 3 - Near future milestone 2023-05-09 16:41:32 +02:00

Sign in to join this conversation.