High performance API for wire signals

[Nightinggale]

TL;DR

Add an easy to use API to allow modded structures to read and write wire signals with a performance, which is comparable to vanilla wire signal handling and won't mess up blueprints.

What ?

When reading the threads for the individual mods, it becomes apparent that wire connections leaves something to be desired. I will summaries the current problems in/for mods as the following:

• Way too CPU hungry when reading wire signals or writing signals to a wire
• Not running each tick even if it makes sense (due to CPU time)
• Output is a constant combinator, meaning it has a limited amount of signal slots (crashes on overflow)
• Modded combinators crashes when writing wire signal, which overflows 32 bit signed ints (but vanilla doesn't?, not very important as the high number was my faulty wiring)
• Wires are attached to invisible entities on top of the main entity, which often breaks blueprints (also claimed to be a source of slowdowns)

I view this as the biggest issue with Factorio. What prevents Factorio from being just another game is the ability to automate using belts and wire signals. There are some great mods for this and some even greater ones, which haven't been made (yet) due to the issues I have mentioned here.

Possible solution
Add a wireAttachmentTile class.
This class is on creation attached to the building entity. It has the tile it attach to (X,Y), graphical offsets in that tile (either with a default or default in documentation) and if it is input or output. When creating a new instance of wireAttachmentTile, the modder is responsible to store the handle/reference/pointer as member data in the building for later access. This removes the need for invisible wire attachment entities and if the API will ensure the wire attachments (including attached wires) will just work with blueprints, then it will be a lot easier to make working wire attachments.

All read/write wire signal functions should be member functions of wireAttachmentTile. This would need to be optimized for performance. Naturally it should be able to read and write arrays of signal <name,value> pairs. However sometimes you don't need the array. What if your modded combinator has "if coal > 0"? A read function to read the int value of just one signal could be useful, particularly if it performs better. Getting a reduced list by requesting all signals higher or lower than an argument could also be useful, again particularly if it performs better (like only positive signals). Reading a list of signals where the list is given as argument(s) also comes to mind if it helps performance.

Writing to a wire could be split into the following functions:

• clear
• write list of <name,value> signal pairs
• write one <name,value> pair
• copy signals from an input given as argument
• write output to wire

Simply put, this allows writing to a buffer using different approaches and then use #5 to update the wire. Not calling #5 would allow the output to be the same as last time, which is important if the combinator doesn't fire every single tick. #4 has the potential to be faster because the wire data never passes through the API and is kept inside exe file. In fact #4 can be used to connect tile 1 to 2 if condition on tile 3 is true, making it a modded decider combinator with control and data on two different wires. Needless to say this should preferably be executed with near decider combinator performance.

To make this API modder friendly and flexible, it would be preferable if read/write functions can take an argument for accessing green/red/both wires (default both). Also for input/output state, the ability to set one as input and one as output could be interesting as it would allow one tile combinators. The ability for the modder to change input/output state of a wire connector should be considered (like flip input/output on a one tile combinator, possibly from GUI). However it's not interesting to support both input and output on the same wire. Some modded combinators does that at the moment and they quickly becomes really annoying to work with.


This is my proposed ideas on how to deal with what I consider the biggest problem in the game. My lack of knowledge of the internal workings of signals makes it hard for me to tell if it's the best solution from a performance point of view, but from a modder coding point of view, this approach seems to be easy to deal with, hence reducing the risk of half working mods, or mods, which aren't blueprint compatible.

The only other recurring issue for making new combinators/entities is making a GUI, but that's a whole different issue, one I think could be solved with examples rather than API changes. Releasing the GUI code for the vanilla combinators would likely do wonders for people making their first GUI, particularly because it allows copying and then have a working GUI to modify to fit the needs.

[Nightinggale]

Copied from [MOD 0.14] AAI Programmable Vehicles viewtopic.php?f=93&t=38475&start=460

Sure there are performance issues regarding adding more signals, but how is performance for one modded combinator vs 5 vanilla ones?

Vanilla vs modded combinator speed, I'm not sure what the exact value is but the ballpark is closer to 50000 than 5.

Also, the actual calculations can be done quickly, it's reading each incoming signal and writing each outgoing signal that takes the most time. So for example, custom combinator plugged into you logistic system would be way slower due to reading lots of signals.

A case where you only write the output if the input changes only solves half the problem because just reading the incoming signal and looking for a change is also quite inefficient. For this reason most modded combinator-like systems are restricted to only run once per second.

I wonder why there is no API hook for this, so the lua code only gets called when the signal input changes. I'm pretty sure the vanilla combinators work that way (and they flash a nice LED on every update too). Checking the signals on every tick seems pretty braindead.

Particularly the last part is a very good point. Why is modded signal handling based on polling instead of calls when data changes? It's an even more relevant question when you consider that polling seems to be the major performance issue.

It seems simple at first. Just add a lua function it will call on signal change. However what if there is more than one wireAttachmentTile instance in the building? To make it simple to use for modders, simply add a function to wireAttachmentTile, which tells if input is changed. It would make sense to use bitmapping, as in 1 = red, 2 = green, 3 = both. If you want to use it as a bool, simply check if it's different from 0. This will make it really easy from a coding point of view to tell where something changed and fetching a cached int shouldn't be slow. It could still be possible to make a function call once a tick if any input changed, but then it will be possible to locate which of the inputs actually did the change.

Expanding on this idea, each wireAttachmentTile can set which signals to listen to, ideally for each wire if needed. Say it needs to check if A > B, then calling a function with a list containing A and B will make the data changed signal trigger on only A or B changes. D can then change, but since it's irrelevant info, the modded combinator will not waste time. This should be quick to change on the fly. Think a decider combinator, which outputs everything. It needs to listen for everything except when it's in false condition, in which case it only needs to listen for the trigger signal(s).

[Optera]

Having an event on_circuit_signal_changed() would be great for modded combinator performance.
Polling in on_tick is a huge waste of performance. Especially if you have multiple mods doing that.


Another big performance drain with signal handling is having to read green and red wire separately and merge them in lua.
Currently every mod reading from both wire colors has to do this in slow lua:

Code: Select all

local function getCircuitValues(entity)
  local greenWire = entity.get_circuit_network(defines.wire_type.green)
  local redWire =  entity.get_circuit_network(defines.wire_type.red)
  local items = {}
  if greenWire and greenWire.signals then
    for _, v in pairs(greenWire.signals) do
      if v.signal.type ~= "virtual" or ControlSignals[v.signal.name] then
        items[v.signal.type..","..v.signal.name] = v.count
      end
    end
  end
  if redWire and redWire.signals then
    for _, v in pairs(redWire.signals) do
      if v.signal.type ~= "virtual" or ControlSignals[v.signal.name] then
        if items[v.signal.type..","..v.signal.name] ~= nil then
          items[v.signal.type..","..v.signal.name] = items[v.signal.type..","..v.signal.name] + v.count
        else
          items[v.signal.type..","..v.signal.name] = v.count
        end
      end
    end
  end
  return items
end

A shortcut, as it exists with for many other functions, would considerably speed up this process.
LuaControlBehavior.Read_Circuit_Signals() -> merged red & green signals or nil if not connected


Writing to Constant Combinators doesn't seem to impact performance nowhere near as much as polling and reading.

[Rseding91]

Having an event on_circuit_signal_changed() would be great for modded combinator performance.

No it wouldn't. It would be called multiple times per tick... way more often than on_tick does now. It would also be desync/crash city.

Currently every mod reading from both wire colors has to do this in slow lua:

Code: Select all

local function getCircuitValues(entity)
  local greenWire = entity.get_circuit_network(defines.wire_type.green)
  local redWire =  entity.get_circuit_network(defines.wire_type.red)
  local items = {}
  if greenWire and greenWire.signals then
    for _, v in pairs(greenWire.signals) do
      if v.signal.type ~= "virtual" or ControlSignals[v.signal.name] then
        items[v.signal.type..","..v.signal.name] = v.count
      end
    end
  end
  if redWire and redWire.signals then
    for _, v in pairs(redWire.signals) do
      if v.signal.type ~= "virtual" or ControlSignals[v.signal.name] then
        if items[v.signal.type..","..v.signal.name] ~= nil then
          items[v.signal.type..","..v.signal.name] = items[v.signal.type..","..v.signal.name] + v.count
        else
          items[v.signal.type..","..v.signal.name] = v.count
        end
      end
    end
  end
  return items
end

I can see several ways that's just sub-optional Lua: all the repeat string concatenation should be done once and stored as a local - as it is now every line re-executes the string concatenation re-creating the string in Lua and causes the Lua parser to re-evaluate each line as it runs. That aside, that's exactly what the game logic does: read red signals then read green signals and do logic that it needs to do.

Writing to Constant Combinators doesn't seem to impact performance nowhere near as much as polling and reading.

Writing constant combinators is a 1-1 match for core game performance because it's the core game that does the logic.

All of the "slow" anyone sees is purely because Lua places ease-of-use above performance and as such people write code like above and don't think anything about the performance implications of what it's actually doing.

[Rseding91]

• Output is a constant combinator, meaning it has a limited amount of signal slots (crashes on overflow)
• Modded combinators crashes when writing wire signal, which overflows 32 bit signed ints (but vanilla doesn't?, not very important as the high number was my faulty wiring)

If you're getting crashes you should report them so they can be fixed. Silently working around them doesn't benefit anyone

Regarding the rest of your idea: that's almost verbatim what the constant combinator entity does now. The circuit network works off signals that are owned by some entity. If you want to have something connected to the network it needs to own the signals that it's sending - which is what the constant combinator does.

You're not going to get better performance than what the constant combinator does now when it comes to mods.

[Optera]

All of the "slow" anyone sees is purely because Lua places ease-of-use above performance and as such people write code like above and don't think anything about the performance implications of what it's actually doing.

I agree lua sucks for doing such things. That's why an API providing the merge would always be faster.
Should be a matter of a few hours if not minutes considering you have to have that merge written for combinators, inserters, asf already internally.

[Nightinggale]

All of the "slow" anyone sees is purely because Lua places ease-of-use above performance and as such people write code like above and don't think anything about the performance implications of what it's actually doing.

No offense to modders, but somehow I find this extremely likely. My experience with modders in general is that the majority seems ok with putting code together to get the result, but completely ignores performance and cache. Once I optimized a function and reduced CPU time by more than 99% (it essentially became a cache lookup instead of nested loop calculation), the person who wrote it was like "Thanks. I didn't know you could do that". This was C++, meaning lua is not the sole culprit, but it does bring up an interesting point. The end result is that mods are still slow, which needs to be fixed. If it can't be fixed with an API update, then it needs to be fixed by solving the issue, which appears to be poor code awareness among the modders.

Could you make an official simple template conbinator mod?
Something simple where it just reads the input and writes the output, but done in a way where it's optimized as much as possible in lua. The sprite code and wire connection points should be coded simple and not like in vanilla where multiple combinators are in the same file and mixed together with sprite changes. Comment why everything is written as it is written and how to use the cached wire input. This will serve a dual purpose. One is a template people can download and modify into whatever they want a combinator to do and the other is documentation on how to do it yourself. Adding a GUI with on on/off switch would likely be a good idea because the forum also has a number of posts regarding people confused on how to start their own GUI window.

It doesn't matter to the players if the slowdown is caused by poor API or poor code in mods, it will still be slow and result in a hot computer.

Regarding the rest of your idea: that's almost verbatim what the constant combinator entity does now. The circuit network works off signals that are owned by some entity. If you want to have something connected to the network it needs to own the signals that it's sending - which is what the constant combinator does.

I still have two unanswered questions though.

1. Blueprints
   Somehow mods have a tendency to not work with blueprints.
   Even if it's poor lua code, those mods are released and annoys the players. If modders can't seem to get blueprint support right, it's either because it's too complex or because they aren't informed how to use it in an easy approach. This could be solved by explaining blueprint support in the template combinator.
2. Max output slots
   Constant combinators have a limited amount of slots to place signals and it crashes if the mod tries to add more. In contrast vanilla combinators seems to have unlimited output slots. This can be solved by assigning say 5000 slots or better yet, the number of slots is set to the number of signals. This brings up some possible issues though.
   A: how will it affect performance and memory usage when it has way too many slots?
   B: how do you assign it to the number of signals during setup?
   A template combinator could assign slots to number of signals, hence removing the issue if people use the template.

If you're getting crashes you should report them so they can be fixed. Silently working around them doesn't benefit anyone

I did. I reported to the mod, which crashed. It was fixed and a new version was uploaded. Turned out it was an overflow in the API and to get around that the value was capped. I admit that at the time I didn't consider if I should report that as an API bug. It would feel more natural to be done by the person, who wrote the code and knew precisely which function was the issue.