Lights added via LuaRendering have FPS impact that persists even after they are cleared by parent entity removal

[Reika]

So this is a bit complex to explain but simple in concept.

I have a save which had the FPS suddenly (in the span of a couple hours, with no major changes to the map) go from 60 to 45. Most of that seemed to be spent on "render preparation" (the first of the three "preparation" entries in the debug "show time used" option).
.

Interestingly, it vanished entirely when in map view.

After some testing, I was able to confirm it to be the lights from my Bioluminescence mod, which uses LuaRendering.draw_light extensively (one tied to each generated glowing plant, of which there are a few per chunk, and one tied to every spawned biter/spitter).

Here is where what I believe is a bug comes in: LuaRendering.clear() removes the lag - FPS immediately returns to 60 - but killing the entities with script/commands does not, even though the lights are removed when their parent entities are:


Further cementing this, using rendering.clear() and then re-creating the lights anew does NOT bring back the lag; FPS is reasonably stable around 60.

This strongly implies that lights created via LuaRendering have some computational overhead that persists even after they are culled through the removal of their parent entity.

This overhead persists across save-load and game reboot cycles. Something tells me the "Each render object is identified by an id that is universally unique for the lifetime of a whole game." in the LuaRendering docs is related.

In the case of my mod, my guess is that the lights tied to biters continued to accumulate this overhead, as there is a near infinite stream of new biters, and removing (ie killing) the old ones did not clear the overhead from their lights.

For now, I will just periodically flush the rendering cache and create new ones, but given that that too probably has potential issues, I would very much prefer if the underlying problem here was resolved.

In case it helps, here is the source of the mod in question, and I have attached the save file. The commands I ran to test were:

• /c for _,e in pairs(game.surfaces[1].find_entities_filtered{type = {"tree", "simple-entity"}}) do e.destroy() end
  
  /c rendering.clear("Bioluminescence")

Upon running the first, you should notice the various glowing plants disappear, along with their lights, and the FPS not improving. The second will improve the FPS.

Here is a command to recreate new lights (with incorrect colors, but who cares for testing purposes) and see that the lag does not return:

/c for _,e in pairs(game.surfaces[1].find_entities_filtered{type = {"tree", "simple-entity"}}) do if string.find(e.name, "glowing", 1, true) then rendering.draw_light{sprite="utility/light_medium", scale=0.5, intensity=1, color={r = 1, g = 1, b = 0, a = 1}, target=e, surface=e.surface} end end

[posila]

I've looked into it, and it is just not optimized for your use case.

Script redering system doesn't have any spacial organization so during rendering every object is checked for validity and if it collides with viewport, so that's the FPS impact.

If the rendering object is bound to an entity and the entity is destroyed, the rendering object is not destroyed immediatelly, it just becomes invalid (because it points to entity that doesn't exist anymore). The script rendering system checks 1 object per mod per tick for validity and deletes it if invalid. So when lot of objects become invalid at the same time, it will take a while before they are actually deleted.
The 1 object per mod may be too tight limit, maybe we could increase it to something like 10.

So, this is not a bug. But there might be some optimization opportunities so I'll move it to modding requests.

In future, I'd like to make graphics definitions on entities more flexible, but I am still mostly experimenting with how it should work - the mining drill and beacon being the latest experiments.

[Reika]

I've looked into it, and it is just not optimized for your use case.

Script redering system doesn't have any spacial organization so during rendering every object is checked for validity and if it collides with viewport, so that's the FPS impact.

If the rendering object is bound to an entity and the entity is destroyed, the rendering object is not destroyed immediatelly, it just becomes invalid (because it points to entity that doesn't exist anymore). The script rendering system checks 1 object per mod per tick for validity and deletes it if invalid. So when lot of objects become invalid at the same time, it will take a while before they are actually deleted.
The 1 object per mod may be too tight limit, maybe we could increase it to something like 10.

So, this is not a bug. But there might be some optimization opportunities so I'll move it to modding requests.

In future, I'd like to make graphics definitions on entities more flexible, but I am still mostly experimenting with how it should work - the mining drill and beacon being the latest experiments.

For my purposes the simplest fix conceptually is to add "light" support to more entity types, in particular "simple" entities like simple-entity, tree, and unit. I asked for this back in 2018 but the script lighting system was seen as sufficiently close to mark as "implemented". viewtopic.php?f=65&t=62923&p=382353&hil ... ht#p382353

One other idea is to add a new entity prototype that is ONLY a light, so that the usual script hooks can just spawn that instead of adding a rendering-type light. That seems rather more work than just allowing simple-entity to support a light and spawning that instead, but maybe there is some optimization reason to do so.

[Qon]

I've looked into it, and it is just not optimized for your use case.

Script redering system doesn't have any spacial organization so during rendering every object is checked for validity and if it collides with viewport, so that's the FPS impact.

If the rendering object is bound to an entity and the entity is destroyed, the rendering object is not destroyed immediatelly, it just becomes invalid (because it points to entity that doesn't exist anymore). The script rendering system checks 1 object per mod per tick for validity and deletes it if invalid. So when lot of objects become invalid at the same time, it will take a while before they are actually deleted.
The 1 object per mod may be too tight limit, maybe we could increase it to something like 10.

So, this is not a bug. But there might be some optimization opportunities so I'll move it to modding requests.

In future, I'd like to make graphics definitions on entities more flexible, but I am still mostly experimenting with how it should work - the mining drill and beacon being the latest experiments.

My use case is somewhat similar and I also have issues with rendering.destroy(). It's so extreme that I'm thinking of the same extreme measures as Reika, rendering.clear() and then rebuild everything whenever I need to destroy just some of them. But I also have huge immediate problems with rendering.destroy(). It seems (with some rudimentary testing) like it takes 10-100 times more time than rendering.draw_line(). If you have 100k lines and want to remove 10k, the fastest way top perform the operation is to clear() and redo all the 9k rendering objects you want to keep. And if you have millions of lines it might still be true that clear() and millions of draw_line() are faster than 10k destroy() calls.

Also with 100k objects destroyed it's going to take half an hour to free up (with 1 per tick) what is destroyed in seconds (even though destroy is so rate limited). My mod ChunkyChunks draws grid lines on the ground. And with several players toggling the grid (which triggers destroy for their individual world covering grids) the cleanup is basically guaranteed to reach hundreds of hours to complete after a few minutes of playtime. I just estimating something like a rate of maybe 3 toggles * 5k render objects /player/minute. That's easily 100k/minute increase with a tiny 3600 cleanups (or 36k) per minute.

Just a single player playing with the toggle button and with grids requiring 100k lines the server is just going to limp forever after that.

100k lines is instant to create or clean. I'm ok with supplying a group id for allocations and destructions to help the garbage collector out if necessary, because the renderings belong to players and are part of specific structures that are always destroyed together (each player has several grids, any adjustment to a grid means destroying all the lines belonging to that grid and making a new one).

I'm trying to implement a toggle hotkey feature for ChunkyChunks, but it seems like using destroy() is possibly doomed.

[Bilka]

I'm trying to implement a toggle hotkey feature for ChunkyChunks, but it seems like using destroy() is possibly doomed.

You could try simply hiding the grid instead, using the set_visible() function. That way no cleanup needs to be done/there are no useless objects hanging around.

But in the end, any operation using id's is O(n), where n is the number of objects (including both destroy(id) and set_visible(id)). So things wont get much faster than using set_visible unless you reduce the number of objects, such as by using the same 32x32 grid for all players and making it visible only for those that have it on (see set_players()).

On the other hand, you could use beams - this means no cheap moving/teleportation of the lines, more expensive creation, less runtime-settable attributes, but cheap access for destruction and rendering.

[Qon]

I'm trying to implement a toggle hotkey feature for ChunkyChunks, but it seems like using destroy() is possibly doomed.

You could try simply hiding the grid instead, using the set_visible() function. That way no cleanup needs to be done/there are no useless objects hanging around.

Right, I had kind of dismissed that since I was on track to use the same feature for toggling on/off as I was for changing grids. Thanks for the reminder, that would work for toggling grids. Changing grid parameters wouldn't work for that, but that happens much less often since it's not needed that often and not as easy to do anyways. Requires that I implement some other optimisations and logic to handle that, but it should work and it's already planned anyways.

But in the end, any operation using id's is O(n), where n is the number of objects (including both destroy(id) and set_visible(id)). So things wont get much faster than using set_visible unless you reduce the number of objects, such as by using the same 32x32 grid for all players and making it visible only for those that have it on (see set_players()).

Right. That was why I suggested allocation groups, so that grids wouldn't be O(n) in individual lines but in much larger collections of hundreds or thousands of lines. I'm splitting the world into 1e4x1e4 sized superchunks that are containing the lines for the grids (which are customizable to any size, though with a limitation to be >= 16 to avoid catastrophic permormance issues). And they are all almost containing the same amount of render objects, so allocation groups would reduce the impact to close to clear(). Hard to say for me if that would be a lot of work to implement though.

Lines at length 1e5 and definitly 1e6 had noticable performance impact on rendering times and dropping my UPS count significatly when visible at least. Same count of lines, just longer. If it wasn't so then I would have drawn my lines from -2e6 to +2e6 (or whatever the max map size was) to reduce line count and improve perfomance of my code significantly. I'm curious why that is, seems like it should be possible for the engine to clip them to the viewport while rendering. Maybe it just isn't done yet and that's why I'm dropping frames then? But that doesn't sound right either.

I've considered player grid sharing, and it is likely that players would use similar sizes. But they can choose different ones, and even with the same sizes they can choose their own individual colouring scheme. So while that is something that I've planned to do and how, it's something that might only have a marginal impact in practise.

The 32x32 grid is probably very common though, and it's pretty small (requires more lines) so is worth optimising for. If I provide a standard 32x32 grid without additional customization in addition to the other more customizable grids, then players might opt to use the standard grid even if it's only available in black and using a customizable grid in 32x32 is also possible.

On the other hand, you could use beams - this means no cheap moving/teleportation of the lines, more expensive creation, less runtime-settable attributes, but cheap access for destruction and rendering.

I'll look into it, might be worth using instead of grid share for the 32x32 grids. If these can be shown for selected individual players only.

I'm considering doing my own in-lua allocation and just re-setting all attibutes to lines that are up for destruction. I'm already spreading destroy() over ticks on ids in separate data structures so I could just use take those when I need to create lines whenever I need to do both destruction and creation (and delay destruction a bit more in anticipation of re-use). I would have to change every single property of the lines though to transform them into something else, is there a performance hit on that? As in does anything actually get restructured immediately between calls or is it essentially a single action if it happens within the same event handler? There's no multi-property assign call that I can see and draw_line() doesn't take an ID as input for re-use.

I guess it's trivial to make a lua wrapper myself for draw_line() that can also optionally take an ID to reuse instead. If that is good for performance.

[Bilka]

I would have to change every single property of the lines though to transform them into something else, is there a performance hit on that? As in does anything actually get restructured immediately between calls or is it essentially a single action if it happens within the same event handler?

The benefit of the current LuaRendering implementation is that changing an attribute doesn't need any internal restructuring. So, you can double the size of your rectangle or teleport it from one end of the world to the other and there is 0 inherent cost to that. Entities have that "restructuring" cost that I mentioned for the beam, but it means cheaper rendering etc.
However, as I said above, any operation using IDs, such as set_visible(id, ...) or set_color(id, ...) etc etc is O(n), where n is the number of objects. So, you'll be doing that O(n) lookup of ID -> object for every single method you use to set a line property (set_color() etc).

[eradicator]

Right. That was why I suggested allocation groups, so that grids wouldn't be O(n) in individual lines but in much larger collections of hundreds or thousands of lines.

Isn't that what draw_polygon() essentially does? As it is a single object with only one id. Ofc you then have to draw the lines connected, but that shouldn't be a problem for a grid pattern.
@Bilka: Or is there any difference in render performance between a 10k-vertices polygon and 10k seperate lines?

[Bilka]

Right. That was why I suggested allocation groups, so that grids wouldn't be O(n) in individual lines but in much larger collections of hundreds or thousands of lines.

Isn't that what draw_polygon() essentially does? As it is a single object with only one id. Ofc you then have to draw the lines connected, but that shouldn't be a problem for a grid pattern.
@Bilka: Or is there any difference in render performance between a 10k-vertices polygon and 10k seperate lines?

It'll probably set your GPU on fire when you try to render it but yes technically that's only one object.

There are some more obvious possible optimisations to make the ID -> object lookup cheaper on the C++ side, but right now the wiki is taking up all my time.

[eradicator]

It'll probably set your GPU on fire when you try to render it but yes technically that's only one object.

Could you give some more technical details on that please? Is it because it has to allocate a texture to contain the whole object or something else? I.e. does that mean even for simple "+" shapes it would be significantly cheaper to render them as 2 lines instead of one 5-vertices polygon (i.e. one "+" per chunk)?

[Bilka]

It'll probably set your GPU on fire when you try to render it but yes technically that's only one object.

Could you give some more technical details on that please? Is it because it has to allocate a texture to contain the whole object or something else? I.e. does that mean even for simple "+" shapes it would be significantly cheaper to render them as 2 lines instead of one 5-vertices polygon (i.e. one "+" per chunk)?

It will draw a single primitive with 10k vertices (https://docs.microsoft.com/en-us/window ... gle-strips). Since that is always on screen, it will always draw the whole thing. I have no personal experience with what happens when you draw such a huge primitive, but Qon's issue with large lines suggests that it wont go well. If you do lines, only some of those are on screen, so only some are rendered, not all 10k.

[Qon]

However, as I said above, any operation using IDs, such as set_visible(id, ...) or set_color(id, ...) etc etc is O(n), where n is the number of objects. So, you'll be doing that O(n) lookup of ID -> object for every single method you use to set a line property (set_color() etc).

Right. That makes it a no-no then.
Hash maps are O(1) though. Looping through it all makes most operations way too costly just by having a static objects. What's the point of non-structured data that is cheap to modify if it's expensive to lookup the ID to actually do the modification? At that point you could sort or do recompaction on the whole set of rendering objects in about the same time as doing a lookup. And it's not even O(n) at that point, it's more like O(n*m) where m is the amount of properties you want to alter, since you can't alter all properties with one ID lookup. Though M is always <= than the amount of properties a render object can have so O(n*m) is a bit misleading.

It'll probably set your GPU on fire when you try to render it but yes technically that's only one object.

Could you give some more technical details on that please? Is it because it has to allocate a texture to contain the whole object or something else? I.e. does that mean even for simple "+" shapes it would be significantly cheaper to render them as 2 lines instead of one 5-vertices polygon (i.e. one "+" per chunk)?

It will draw a single primitive with 10k vertices (https://docs.microsoft.com/en-us/window ... gle-strips). Since that is always on screen, it will always draw the whole thing. I have no personal experience with what happens when you draw such a huge primitive, but Qon's issue with large lines suggests that it wont go well. If you do lines, only some of those are on screen, so only some are rendered, not all 10k.

I might have used gap=0 dash=32 for lines at length 2e6. If Factorio doesn't treat that the same as gap=0, dash=0 (continous line) and makes a single line split into ~2^15 segments then that might explain the performance hit. I need to retest. And this time set dash=0 when gap=0.

But I have some locality to lines. I could maybe group like 10 lines that are close together into a single polygon?

I'm not drawing my lines/chunk (assuming 32x32 grid), not even when doing crosses. I'm drawing long lines from top of the area to cover to bottom. Also doing that when doing "+" and smaller rectangles (like 30x30 rectangle within each chunk), by using the dash and gap fields approprietely. That whay I get O(n) lines in side length of the area to cover instead of O(n^2). And also why increasing line length helps performance a lot for me. Size 1e4 doesn't impact rendering times noticably but is at least big enough to get big advantages already.

The grouping I wanted was for memory allocation but not for rendering.

[orzelek]

If you are using triangle strips then it would make sense to merge lines that are nearby into one triangle strip with degenerate triangles. So if lines are close then you can make 10 lines into one polygon.
From what I know (and tested a bit on windows/iOS) polygon that can be rendered in one shot would be preferred to lots of lines. Size doesn't matter that much because clipping is very efficient on GPU. Using 128x128 with mesh pattern should work nicely. And GPU should prefer one 128x128 complex polygon than 16 separate polygon lines.
There is an upper bound on size most likely but it's hard to tell how big you can go. My guess would be that even 1kx1k poly would be fast to render even if your screen is showing 32x32 area atm.. but that depends on many factors.

And making anything dashed will most likely kill performance since dashed lines are pain to render (texture with pattern and calculated vertices or calculated polygons that from your test would seem are not made into one strip).

[Reika]

A quick heads up and a word of warning for those who may be planning to get clever on ways around this issue:

I tried this:

Which, when unchecked, promptly resulted in this:


viewtopic.php?f=7&t=86385&p=504942#p504942

[Eifel]

...
After some testing, I was able to confirm it to be the lights from my Bioluminescence mod, ...

i can confirm that, i now identified (thanks for this thread)that this mods cause fps/ups drops (~25fps/40ups) after removeing this mod i can do 120fps/120ups without problem (with game speed 2)

[Reika]

...
After some testing, I was able to confirm it to be the lights from my Bioluminescence mod, ...

i can confirm that, i now identified (thanks for this thread)that this mods cause fps/ups drops (~25fps/40ups) after removeing this mod i can do 120fps/120ups without problem (with game speed 2)

The mod has settings to use entity-based lights instead. Use that to solve the issue.

[Bilka]

This was optimized in 1.1: Deletion of objects where the entity target is destroyed is now instant. Furthermore, access to objects has been optimized to be O(log(n)) instead of O(n).

With 2.0, access to objects to update properties will be constant time, which is achieved by switching away from the ID system to LuaRenderObjects. This functions similarly to LuaEntities, so hopefully it should be a very familiar system.

[Stringweasel]

Always fun seeing a casual conversation in the Discord improve the base game

[Qon]

This was optimized in 1.1: Deletion of objects where the entity target is destroyed is now instant. Furthermore, access to objects has been optimized to be O(log(n)) instead of O(n).

This will be a big improvement for my mod that sometimes have to individually clear a huge pile of objects! I thought you would stick with plain arrays forever. I assumed you wanted that for linearly processing the data of all LuaRendering objects without overhead. I like the change, but did you have to sacrifice anything? Do you loop through all LuaRendering objects to render them and have worse asymptotic performance now, or is it still O(n) for all with some more overhead, or not? Or do you spatially group them now to avoid processing all every frame to not need fast array loop through for all LuaRender objects? I don't know the actual system so I'm just throwing out questions that might apply from assumptions I made when I thought that you had decided that O(n) for single object removal was good enough. Please feel free to give me some insight in the design and the design process and the actual considerations you balanced.

With 2.0, access to objects to update properties will be constant time, which is achieved by switching away from the ID system to LuaRenderObjects. This functions similarly to LuaEntities, so hopefully it should be a very familiar system.

This is probably usable for performance speedup, after I rewrite my mods. Thanks for this as well , even if I'm not excited about rewriting stuff