Which ABM's could/should be removed? #1561
Comments
|
You are approaching from wrong direction, you should first profile abms on IRL servers, check what is the heaviest and then think about what to do... |
|
How can I profile them? |
|
|
|
I never worked with the profiler. Can you tell me what's next? |
|
enable it in config, load the game/server, in console type |
|
Thanks, that is helpful |
I didn't make that a nodetimer? |
|
if the engine could honor calling on_construct for liquids then the lava ABM can be removed entirely. So, I suggest we make that change. It would also potentially aid protection mods. |
|
Saplings have an |
|
A quick profile shows
Those are going to be the bulk of the ABM cost, and we can't get rid of them I think. |
|
Oops forgot about node timers. |
|
Currently it seems that I have to run minetest.find_nodes_in_area() over a chunk to find e.g. all the flowers and mushrooms and start their node timer. Don't know if that's the best idea |
|
The reason flowers and mushrooms are not nodetimer based is because they're fundamentally different than saplings and farming crops: The growth of those is finite, but mushroom spread should always happen. (same as grass covering dirt). So there is no reason to attempt to convert these. ABM's can continue to serve them as efficient (on a large scale, I'm not talking micro level comparison) as nodetimers can. So, before you attempt to write a lot of ugly code that works around the problems mentioned, keep in mind that if something is supposed to happen all the time, and forever, then ABM's are likely going to be the simplest way to do it. |
|
You are right. It was just a thought game. I don't have the feeling that the flower ABM is that heavy that it needs to be replaced The only ABM that could be replaced is the lava cooling, but I'm not sure if it't worth it because lava mostly found in the underground So possible close of this issue? Cause I don't think we need to replace any other ABM |
|
I've just seen that lavacooling is interval 1 chance 1, which is crazy, and i was the one fiddling with these values, perhaps it can be 2 and 2 instead? Chance 1 can overload the sound generator when a large volume cools. Newer mapgen have lava in larger volumes. |
|
Lava cooling is one of those things that should get fixed radically differently IMHO. First, it's not something that happens to all lava nodes no matter what - it is entirely event driven. It will only ever occur when new lava is created, or when new water is created. The problem is that liquid updates do not call |
|
In general I think, the liquid system needs a rewrite |
|
sofar yes that sounds good let's try it. |
|
Wait what? O_o Lava cooling really has an interval of only 1! Admittedly the callback function is infrequently executed, but the nodes in every active map block are still being checked repeatedly. I threw together a very basic profiler in the ABMHandler class, to see just how many node checks are actually being performed per second with just a single player online. It literally blew my mind! In all fairness, block->getNodeNoEx( ) is lightweight. Yet as shown above, there are over half a million node checks every second, most of which fail and thus are completely wasteful of CPU cycles. That is a lot of needless repetition that could be greatly optimized if a different algorithm were used for ABMs. I guess I should go back and start vetting all ABMs, because this is quite scary to be honest :( |
|
I think, instead of removing the lava cooling ABM the whole liquid system should be changed |
|
Yes, it would seem liquids should be overhauled as well. I think together with ABMs, they can be a severe performance drain even on high-end systems. |
|
If we had an on_flood function active for nodes then we could use that to replace with stone or obsidian only when required instead of all the checks every second. |
|
We are not going to suddenly do a complete rewrite of liquids, it actually works fairly well, it's very easy to be dissatisfied with it, but that's because it's simple and unrealistic for good reason. If you tried to code a liquid system you would find it's much more difficult than you think, then you would realise what we have works quite well. |
|
Here's an idea for a rudimentary alternative to the current ABM mechanism, one which could provide significant advantages in terms of efficiency and simplicity:
Since map blocks are infrequently loaded and/or modified and the trigger conditions for ABMs are already known at the time of registration, then the results can obviously be cached into a simple bitmap (where 1 is pass and 0 is fail). This, of course, is just an ad hoc example. There are undoubtedly more robust algorithms that could be devised to account for all manner of scenarios. I'm mainly putting this out there just to show what can be done to greatly lessen the CPU workload with active block modifiers. |
|
There have been ideas discussed about how to remove the lava ABM, replacing it with an update when nearby nodes are updated. |
However on mapgen lava nodes are generated next to water nodes and there may not be an update. |
|
Reopen as is important and ideas are very welcome. Here's one for flora spreading, that may well not be usable in MTG but is worth noting. I suggest that flora spread is something that can happen when you dig a flora node. Therefore spreading happens when and where it is needed due to the removal of a flora node, instead of happening everywhere, increasing density where a low density is intended, and most of the time doing nothing because the maximum density has been reached. On this player action 1 or 2 nearby soil nodes can be 'seeded' with a seedling of the dug flora node and nodetimers started on them for later growth. The seedlings could be tiny plantlike nodes on the surface. We therefore use a system like tree saplings but automate the seeding so that you don't have to collect seeds and plant them. |
|
MTG ABMs and their intervals: default: farming: tnt: fire: flowers: stairs: 12 ABMs running by default. Intervals ordered, brackets indicate ABMs usually not running: The intention is for each ABM to have a different interval to reduce the occurence of many ABMs acting at once, to make them as out-of-phase as possible. |
|
Of these, flower and mushroom spread could certainly be removed using either seeds or the above described method. But, might cause disruption for some players and worlds. Coral death: EDIT: Done. Coral is only placed underwater in places unlikely to be drained of water (if they were drained players would likely dig the coral anyway). Dug coral drops dead coral. Kelp does not die if drained of water. Many other plants survive in weird situations, like removing dirt from under a tree. Fire: EDIT: Done. 'extinguish flame' (run every 3s so very intensive) can be done by making flames 'floodable = true'. The ABM would be needed for ice and snow to extinguish, but these nodes do not melt in this process so extinguishing doesn't make much sense, there's no meltwater to run into the flame. Note that flame spread already prevents flames appearing next to snow and ice, the ABM is only there for the situation of placing snow or ice to extinguish a flame, the player can extinguish the flame much faster just by punching it. |
|
Maybe instead optimise it engine itself? Don't cut out those minor yet meaningful abms... |
|
We await your PR :) |
|
#2172 merged. |
|
@Ezhh any opinion on removing fire extinguish ABM? #1561 (comment) |
|
fire extinguish ABM is not hugging lot of resources at all but the right approach would be to not spread fire when disabled instead of spreading fire anyway and then removing it |
|
I disagree with removing the fire ABM. With the ABM caching optimisation, ABMs are now very inexpensive if there are no such nodes in a block - which is the case almost all the time for fire |
|
If all the ABM really does is remove fire if it happens to be next to snow/ice nodes, I am struggling to understand the point of it (as opposed to currently thinking about it in terms of an optimisation issue). Fire simply being next to ice or snow would not put out fire, so long as the fire itself has a suitable fuel source, so it seems a bit of a pointless thing. That leaves water, which should put out fire... if it floods the node the fire is in, which does bring us back to on_flood again. Instead of thinking about how resource-hungry this particular ABM is, we should figure out if the current behaviour is actually desired or not. |
|
That ABM optimisation PR was merged, is this issue still relevant? |
I agree with this very much |
|
Fixer yes it's still relevant, because ABMs can still be temporarily intensive (see below), and they are essentially undesirable as they are continuous instead of event-triggered. The newly-added ABM cache is not saved in the database, so each time a block is loaded the ABMs run as before the first time, then afterwards the cache optimisation has effect. When a block is unloaded from the server (this happens often) the cache is lost. It's good that it's less urgent but my attitude to ABMs hasn't changed much, they need to be removed where reasonably possible. As i explained a few posts back, there isn't actually much need for this ABM at all once fire is floodable, so i still think it's worth removing the ABM. Neighbouring snow or ice doesn't melt and isn't removed anyway. |
|
Unless there is something I'm missing, I'd be in favour of this change simply from a common sense standpoint, because I don't think the current behaviour is right. @rubenwardy What's your stance on this? You were against removing the ABM to begin, but if you agree the current snow/ice behaviour isn't needed, on_flood makes more sense to me than the ABM does. |
|
Also consider, whenever the contents of a mapblock are changed the cache is invalidated and the ABM runs as before. Considering that players are very often digging or placing nodes, and mods and are altering the world, this means ABMs are very often running as intensively as before. |
|
PR #2200 |
|
Another ABM that could possibly be removed: EDIT: Done. When fire is disabled there is still a ABM running to remove flames. Instead we could simply remove the flame when it's timer expires. |
Well .. both need to be considered, as the ABM optimisation only occurs in some situations, and not at all during a fire as the cache is constantly being invalidated. The extinguish ABM has very little or no benefit so i don't think it justified. |
|
#2200 merged. |
|
PR #2209 for #1561 (comment) |
|
#2209 merged, excellent we've made some good progress. |
|
It seems we have removed roughly everything that can be, so closing. New issues can be made if further ABMs seem removable. |
There are a lot of ABM's in MTG. One of them could be replaced by something lightweight.
I think, most of the ABM's could be replaced, but the question is: is it worth it?
I tried to replace lava cooling, but it is difficult. Just worked well with lava source, not with lava flowing
The text was updated successfully, but these errors were encountered: