[WIP] Improve calculation of averages #2
Conversation
| if entity_count > entity_rate_max then | ||
| sec_multiplier = entity_rate_max / entity_count | ||
| end | ||
| local sec_value_count = math.ceil(60 * sec_multiplier) |
There was a problem hiding this comment.
eg. if there are 2000 entities, and we can only update 1000 per tick, then sec_multiplier will be 0.5, resulting in 30 data points per second being stored in sec_avg
| add2(sec_avg, data.sec_avg.values[i]) | ||
| end | ||
| data.sec_avg = sec_avg | ||
| global.entity_data[id] = data |
There was a problem hiding this comment.
Is changing global data like this a bad idea? Regardless, this shouldn't happen too often.
There was a problem hiding this comment.
An interesting idea. I think though I would want to gate the recalculation to something like cur_tick % 1800== 0 (every 30 seconds at 60 fps) to prevent a lot of potential recalcs (imagine if something is creating/destroying frequently that results in sec_value_count ping-ponging).
May be better to just adjust sec_avg.total as opposed to rebuilding the array (defining the avg.values table to have the maximum length possible), since if this triggers it'll do a rebuild on every entity evaluated this tick. If being proper you would need to shift elements around to make sure the most recents are still being evaluated.
There was a problem hiding this comment.
Good point, didn't consider just ignoring the additional values with a shorter sec_avg.total
What do you think the best way to implement the 30 second recalc limit? Maybe have a second update marker to track recalc's like we have for updating labels?
There was a problem hiding this comment.
Similar logic to the label updating, except:
if cur_tick % 1800 == 0 then
rethink_secs() or whatever
end
In the on_tick loop (around line 600 or so)
There was a problem hiding this comment.
Thanks! I'll take another look this weekend
| @@ -581,6 +581,8 @@ local function on_tick(event) | |||
| local next = next | |||
| local entity_data = global.entity_data | |||
There was a problem hiding this comment.
Is it a common pattern in factorio modding to clone global data like this? I noticed that in a test world with 28,000 assemblers, the mod uses slightly less cpu time when referencing the global directly instead of cloning in the on_tick handler.
I'm guessing mutations to global are a bit slower because it has to sync with all clients, but maybe for mostly read-only code like this, it might be better to read directly from the global instead of cloning it?
There was a problem hiding this comment.
Advice I got a while ago was to do a copy to local if you're referencing a global variable multiple times, as global accesses are slow. I haven't re-verified this or benchmarked it since before 1.0 though.
jahands
changed the title
On a larger world, I noticed that this mod was using ~1.6ms/tick on average. On megabases, every ms matters when trying to keep TPS at 60.
My first thought was to decrease how many entities were processed per tick, but this causes data to take longer to update because it always stores 60 data points to calculate the last second. Eg. if it's only able to check half of the entities per tick, then 60 data points in sec_avg is actually 2 seconds worth of data.
This PR changes sec_avg to store a dynamic number of data points based on how many ticks it takes to update all entities.