Merge pull request #82 from mamoniot/dev

added a performance cache for SE

cybersyn/changelog.txt

@@ -1,4 +1,9 @@
 ---------------------------------------------------------------------------------------------------
+Version: 1.2.15
+Date: 2023-4-30
+  Bugfixes:
+    - Fixed UPS spikes in Space Exploration related to expensive remote calls into their modding interface.
+---------------------------------------------------------------------------------------------------
 Version: 1.2.14
 Date: 2023-4-30
   Features:

cybersyn/info.lua

@@ -3,4 +3,4 @@
 --- It is used in migrations.lua to determine if any migrations need to be run for beta testers.
 --- It is expected these are only meaningful between releases during beta testing.
 --- It should be set to nil for any release version.
-return nil
+return 1

cybersyn/locale/en/base.cfg

@@ -12,7 +12,7 @@ cybersyn-warmup-time=Station warmup time (sec)
 cybersyn-stuck-train-time=Stuck train timeout (sec)
 cybersyn-allow-cargo-in-depot=Allow cargo in depots
 cybersyn-invert-sign=Invert combinator output (deprecated)
-cybersyn-manager-enabled=Enable the Cybersyn GUI.
+cybersyn-manager-enabled=Preview the Indev Cybersyn Manager.
 cybersyn-manager-update-rate=Manager refresh tick interval
 cybersyn-manager-result-limit=Max entities displayed on GUI pages.
 
@@ -30,6 +30,8 @@ cybersyn-warmup-time=How many seconds a cybernetic combinator will wait before c
 cybersyn-stuck-train-time=After this many seconds from a train's dispatch, an alert will be sent to let you know a train is probably stuck and has not completed its delivery. The player will likely have to debug their network to get the train unstuck.
 cybersyn-allow-cargo-in-depot=If checked, trains will be allowed to have cargo in depots; no alerts will be generated and the train will not be held. In addition, trains with orders to visit requester stations with "Inactivity condition" checked will wait for inactivity instead of waiting for empty cargo. Useful for creating train systems where depots handle excess cargo. For advanced users only.
 cybersyn-invert-sign=Flip the sign of the output of cybernetic combinators to be the same as it is in LTN or in earlier versions of Project Cybersyn.
+cybersyn-manager-enabled=Allows viewing of all trains and stations connected to the Cybersyn network. This GUI is currently in development and is likely to have many crash.
+cybersyn-manager-update-rate=Controls how frequently the Cybersyn manager gui refreshes. The Cybersyn manager must be set to active for this setting to have an effect.
 cybersyn-manager-result-limit=Caps the number of matching enitities (e.g. stations, trains) to limit the amount of update time consumed when the list is refreshed.\n-1 means return all results.
 
 [item-name]

cybersyn/scripts/factorio-api.lua

@@ -33,19 +33,71 @@ function get_dist(entity0, entity1)
 end
 
 
+---@param cache PerfCache
 ---@param surface LuaSurface
-function se_get_space_elevator_name(surface)
+function se_get_space_elevator_name(cache, surface)
-	--TODO: check how expensive the following is and potentially cache it's results
+	---@type LuaEntity?
-	local entity = surface.find_entities_filtered({
+	local entity = nil
-		name = SE_ELEVATOR_STOP_PROTO_NAME,
+	---@type string?
-		type = "train-stop",
+	local name = nil
-		limit = 1,
+	local cache_idx = 2*surface.index
-	})[1]
+	if cache.se_get_space_elevator_name then
+		entity = cache.se_get_space_elevator_name[cache_idx - 1]
+		name = cache.se_get_space_elevator_name[cache_idx]
+	else
+		cache.se_get_space_elevator_name = {}
+	end
+
 	if entity and entity.valid then
-		return string.sub(entity.backer_name, 1, string.len(entity.backer_name) - SE_ELEVATOR_SUFFIX_LENGTH)
+		return name
+	else
+		--Chaching failed, default to expensive lookup
+		entity = surface.find_entities_filtered({
+			name = SE_ELEVATOR_STOP_PROTO_NAME,
+			type = "train-stop",
+			limit = 1,
+		})[1]
+
+		if entity then
+			name = string.sub(entity.backer_name, 1, string.len(entity.backer_name) - SE_ELEVATOR_SUFFIX_LENGTH)
+			cache.se_get_space_elevator_name[cache_idx - 1] = entity
+			cache.se_get_space_elevator_name[cache_idx] = name
+			return name
+		else
+			return nil
+		end
 	end
 end
+---@param cache PerfCache
+---@param surface_index uint
+local function se_get_zone_from_surface_index(cache, surface_index)
+	---@type uint?
+	local zone_index = nil
+	---@type uint?
+	local zone_orbit_index = nil
+	local cache_idx = 2*surface_index
+	if cache.se_get_zone_from_surface_index then
+		zone_index = cache.se_get_zone_from_surface_index[cache_idx - 1]--[[@as uint]]
+		--zones may not have an orbit_index
+		zone_orbit_index = cache.se_get_zone_from_surface_index[cache_idx]--[[@as uint?]]
+	else
+		cache.se_get_zone_from_surface_index = {}
+	end
 
+	if not zone_index then
+		zone = remote.call("space-exploration", "get_zone_from_surface_index", {surface_index = surface_index})
+
+		if zone and type(zone.index) == "number" then
+			zone_index = zone.index--[[@as uint]]
+			zone_orbit_index = zone.orbit_index--[[@as uint?]]
+			--NOTE: caching these indices could be a problem if SE is not deterministic in choosing them
+			cache.se_get_zone_from_surface_index[cache_idx - 1] = zone_index
+			cache.se_get_zone_from_surface_index[cache_idx] = zone_orbit_index
+		end
+	end
+
+	return zone_index, zone_orbit_index
+end
 
 ---@param train LuaTrain
 ---@return LuaEntity?
@@ -249,12 +301,12 @@ function set_manifest_schedule(map_data, train, depot_stop, same_depot, p_stop,
 	elseif IS_SE_PRESENT then
 		local other_surface_i = (not is_p_on_t and p_surface_i) or (not is_r_on_t and r_surface_i) or d_surface_i
 		if (is_p_on_t or p_surface_i == other_surface_i) and (is_r_on_t or r_surface_i == other_surface_i) and (is_d_on_t or d_surface_i == other_surface_i) then
-			local t_zone = remote.call("space-exploration", "get_zone_from_surface_index", {surface_index = t_surface_i})--[[@as {}]]
+			local t_zone_index, t_zone_orbit_index = se_get_zone_from_surface_index(map_data.perf_cache, t_surface_i)
-			local other_zone = remote.call("space-exploration", "get_zone_from_surface_index", {surface_index = other_surface_i})--[[@as {}]]
+			local other_zone_index, other_zone_orbit_index = se_get_zone_from_surface_index(map_data.perf_cache, other_surface_i)
-			if t_zone and other_zone then
+			if t_zone_index and other_zone_index then
-				local is_train_in_orbit = other_zone.orbit_index == t_zone.index
+				local is_train_in_orbit = other_zone_orbit_index == t_zone_index
-				if is_train_in_orbit or t_zone.orbit_index == other_zone.index then
+				if is_train_in_orbit or t_zone_orbit_index == other_zone_index then
-					local elevator_name = se_get_space_elevator_name(t_surface)
+					local elevator_name = se_get_space_elevator_name(map_data.perf_cache, t_surface)
 					if elevator_name then
 						local records = {create_inactivity_order(depot_stop.backer_name)}
 						if t_surface_i == p_surface_i then
@@ -329,28 +381,32 @@ function add_refueler_schedule(map_data, train, stop)
 		train.schedule = schedule
 		return true
 	elseif IS_SE_PRESENT then
-		local t_zone = remote.call("space-exploration", "get_zone_from_surface_index", {surface_index = t_surface_i})--[[@as {}]]
+		local t_zone_index, t_zone_orbit_index = se_get_zone_from_surface_index(map_data.perf_cache, t_surface_i)
-		local other_zone = remote.call("space-exploration", "get_zone_from_surface_index", {surface_index = f_surface_i})--[[@as {}]]
+		local other_zone_index, other_zone_orbit_index = se_get_zone_from_surface_index(map_data.perf_cache, f_surface_i)
-		local is_train_in_orbit = other_zone.orbit_index == t_zone.index
+		if t_zone_index and other_zone_index then
-		if is_train_in_orbit or t_zone.orbit_index == other_zone.index then
+			local is_train_in_orbit = other_zone_orbit_index == t_zone_index
-			local elevator_name = se_get_space_elevator_name(t_surface)
+			if is_train_in_orbit or t_zone_orbit_index == other_zone_index then
-			local cur_order = schedule.records[i]
+				local elevator_name = se_get_space_elevator_name(map_data.perf_cache, t_surface)
-			local is_elevator_in_orders_already = cur_order and cur_order.station == elevator_name..(is_train_in_orbit and SE_ELEVATOR_ORBIT_SUFFIX or SE_ELEVATOR_PLANET_SUFFIX)
+				if elevator_name then
-			if not is_elevator_in_orders_already then
+					local cur_order = schedule.records[i]
-				table_insert(schedule.records, i, se_create_elevator_order(elevator_name, is_train_in_orbit))
+					local is_elevator_in_orders_already = cur_order and cur_order.station == elevator_name..(is_train_in_orbit and SE_ELEVATOR_ORBIT_SUFFIX or SE_ELEVATOR_PLANET_SUFFIX)
-			end
+					if not is_elevator_in_orders_already then
-			i = i + 1
+						table_insert(schedule.records, i, se_create_elevator_order(elevator_name, is_train_in_orbit))
-			is_train_in_orbit = not is_train_in_orbit
+					end
-			table_insert(schedule.records, i, create_inactivity_order(stop.backer_name))
+					i = i + 1
-			i = i + 1
+					is_train_in_orbit = not is_train_in_orbit
-			if not is_elevator_in_orders_already then
+					table_insert(schedule.records, i, create_inactivity_order(stop.backer_name))
-				table_insert(schedule.records, i, se_create_elevator_order(elevator_name, is_train_in_orbit))
+					i = i + 1
-				i = i + 1
+					if not is_elevator_in_orders_already then
-				is_train_in_orbit = not is_train_in_orbit
+						table_insert(schedule.records, i, se_create_elevator_order(elevator_name, is_train_in_orbit))
-			end
+						i = i + 1
+						is_train_in_orbit = not is_train_in_orbit
+					end
 
-			train.schedule = schedule
+					train.schedule = schedule
-			return true
+					return true
+				end
+			end
 		end
 	end
 	--create an order that probably cannot be fulfilled and alert the player
@@ -358,6 +414,7 @@ function add_refueler_schedule(map_data, train, stop)
 	lock_train(train)
 	train.schedule = schedule
 	send_alert_cannot_path_between_surfaces(map_data, train)
+	return false
 end
 
 

cybersyn/scripts/global.lua

@@ -24,6 +24,11 @@
 ---@field public each_refuelers {[uint]: true}
 ---@field public active_alerts {[uint]: {[1]: LuaTrain, [2]: int}}?
 ---@field public manager Manager
+---@field public perf_cache PerfCache -- This gets reset to an empty table on migration change
+
+---@class PerfCache
+---@field public se_get_space_elevator_name {}?
+---@field public se_get_zone_from_surface_index {}?
 
 ---@class Station
 ---@field public entity_stop LuaEntity
@@ -161,6 +166,7 @@ function init_global()
 	global.refuelers = {}
 	global.to_refuelers = {}
 	global.each_refuelers = {}
+	global.perf_cache = {}
 
 	IS_SE_PRESENT = remote.interfaces["space-exploration"] ~= nil
 end

cybersyn/scripts/migrations.lua

@@ -326,6 +326,8 @@ local migrations_table = {
 function on_config_changed(data)
 	global.tick_state = STATE_INIT
 	global.tick_data = {}
+	global.perf_cache = {}
+
 	flib_migration.on_config_changed(data, migrations_table)
 
 	for i, v in pairs(global.manager.players) do
@@ -349,25 +351,4 @@ end
 ---NOTE 2: Everything in this section must be idempotent
 function on_debug_revision_change()
 	local map_data = global
-
-	if debug_revision == 1 then
-		for _, e in pairs(map_data.refuelers) do
-			if e.network_flag ~= nil then
-				e.network_mask = e.network_flag
-				e.network_flag = nil
-			end
-		end
-		for _, e in pairs(map_data.stations) do
-			if e.network_flag ~= nil then
-				e.network_mask = e.network_flag
-				e.network_flag = nil
-			end
-		end
-		for _, e in pairs(map_data.trains) do
-			if e.network_flag ~= nil then
-				e.network_mask = e.network_flag
-				e.network_flag = nil
-			end
-		end
-	end
 end