Merge pull request #82777 from Faless/mp/perfs

[MP] Various performance optimizations

modules/multiplayer/multiplayer_debugger.cpp

@@ -239,8 +239,8 @@ void MultiplayerDebugger::RPCProfiler::tick(double p_frame_time, double p_proces
 MultiplayerDebugger::SyncInfo::SyncInfo(MultiplayerSynchronizer *p_sync) {
 	ERR_FAIL_NULL(p_sync);
 	synchronizer = p_sync->get_instance_id();
-	if (p_sync->get_replication_config().is_valid()) {
-		config = p_sync->get_replication_config()->get_instance_id();
+	if (p_sync->get_replication_config_ptr()) {
+		config = p_sync->get_replication_config_ptr()->get_instance_id();
 	}
 	if (p_sync->get_root_node()) {
 		root_node = p_sync->get_root_node()->get_instance_id();

modules/multiplayer/multiplayer_synchronizer.cpp

@@ -441,6 +441,10 @@ List<NodePath> MultiplayerSynchronizer::get_delta_properties(uint64_t p_indexes)
 	return out;
 }
 
+SceneReplicationConfig *MultiplayerSynchronizer::get_replication_config_ptr() const {
+	return replication_config.ptr();
+}
+
 MultiplayerSynchronizer::MultiplayerSynchronizer() {
 	// Publicly visible by default.
 	peer_visibility.insert(0);

modules/multiplayer/multiplayer_synchronizer.h

@@ -118,6 +118,7 @@ class MultiplayerSynchronizer : public Node {
 
 	List<Variant> get_delta_state(uint64_t p_cur_usec, uint64_t p_last_usec, uint64_t &r_indexes);
 	List<NodePath> get_delta_properties(uint64_t p_indexes);
+	SceneReplicationConfig *get_replication_config_ptr() const;
 
 	MultiplayerSynchronizer();
 };

modules/multiplayer/scene_cache_interface.cpp

@@ -44,9 +44,8 @@ void SceneCacheInterface::on_peer_change(int p_id, bool p_connected) {
 		path_get_cache.erase(p_id);
 		// Cleanup sent cache.
 		// Some refactoring is needed to make this faster and do paths GC.
-		for (const KeyValue<NodePath, PathSentCache> &E : path_send_cache) {
-			PathSentCache *psc = path_send_cache.getptr(E.key);
-			psc->confirmed_peers.erase(p_id);
+		for (KeyValue<ObjectID, PathSentCache> &E : path_send_cache) {
+			E.value.confirmed_peers.erase(p_id);
 		}
 	}
 }
@@ -67,7 +66,7 @@ void SceneCacheInterface::process_simplify_path(int p_from, const uint8_t *p_pac
 	String paths;
 	paths.parse_utf8((const char *)(p_packet + ofs), p_packet_len - ofs);
 
-	NodePath path = paths;
+	const NodePath path = paths;
 
 	if (!path_get_cache.has(p_from)) {
 		path_get_cache[p_from] = PathGetCache();
@@ -81,7 +80,7 @@ void SceneCacheInterface::process_simplify_path(int p_from, const uint8_t *p_pac
 	}
 
 	PathGetCache::NodeInfo ni;
-	ni.path = path;
+	ni.path = node->get_path();
 
 	path_get_cache[p_from].nodes[id] = ni;
 
@@ -106,29 +105,34 @@ void SceneCacheInterface::process_simplify_path(int p_from, const uint8_t *p_pac
 
 void SceneCacheInterface::process_confirm_path(int p_from, const uint8_t *p_packet, int p_packet_len) {
 	ERR_FAIL_COND_MSG(p_packet_len < 3, "Invalid packet received. Size too small.");
+	Node *root_node = SceneTree::get_singleton()->get_root()->get_node(multiplayer->get_root_path());
+	ERR_FAIL_NULL(root_node);
 
 	const bool valid_rpc_checksum = p_packet[1];
 
 	String paths;
 	paths.parse_utf8((const char *)&p_packet[2], p_packet_len - 2);
 
-	NodePath path = paths;
+	const NodePath path = paths;
 
 	if (valid_rpc_checksum == false) {
 		ERR_PRINT("The rpc node checksum failed. Make sure to have the same methods on both nodes. Node path: " + path);
 	}
 
-	PathSentCache *psc = path_send_cache.getptr(path);
+	Node *node = root_node->get_node(path);
+	ERR_FAIL_NULL(node);
+
+	PathSentCache *psc = path_send_cache.getptr(node->get_instance_id());
 	ERR_FAIL_NULL_MSG(psc, "Invalid packet received. Tries to confirm a path which was not found in cache.");
 
 	HashMap<int, bool>::Iterator E = psc->confirmed_peers.find(p_from);
 	ERR_FAIL_COND_MSG(!E, "Invalid packet received. Source peer was not found in cache for the given path.");
 	E->value = true;
 }
 
-Error SceneCacheInterface::_send_confirm_path(Node *p_node, NodePath p_path, PathSentCache *psc, const List<int> &p_peers) {
+Error SceneCacheInterface::_send_confirm_path(Node *p_node, PathSentCache *psc, const List<int> &p_peers) {
 	// Encode function name.
-	const CharString path = String(p_path).utf8();
+	const CharString path = String(multiplayer->get_root_path().rel_path_to(p_node->get_path())).utf8();
 	const int path_len = encode_cstring(path.get_data(), nullptr);
 
 	// Extract MD5 from rpc methods list.
@@ -163,8 +167,9 @@ Error SceneCacheInterface::_send_confirm_path(Node *p_node, NodePath p_path, Pat
 	return err;
 }
 
-bool SceneCacheInterface::is_cache_confirmed(NodePath p_path, int p_peer) {
-	const PathSentCache *psc = path_send_cache.getptr(p_path);
+bool SceneCacheInterface::is_cache_confirmed(Node *p_node, int p_peer) {
+	ERR_FAIL_NULL_V(p_node, false);
+	const PathSentCache *psc = path_send_cache.getptr(p_node->get_instance_id());
 	ERR_FAIL_NULL_V(psc, false);
 	HashMap<int, bool>::ConstIterator F = psc->confirmed_peers.find(p_peer);
 	ERR_FAIL_COND_V(!F, false); // Should never happen.
@@ -174,13 +179,13 @@ bool SceneCacheInterface::is_cache_confirmed(NodePath p_path, int p_peer) {
 int SceneCacheInterface::make_object_cache(Object *p_obj) {
 	Node *node = Object::cast_to<Node>(p_obj);
 	ERR_FAIL_NULL_V(node, -1);
-	NodePath for_path = multiplayer->get_root_path().rel_path_to(node->get_path());
+	const ObjectID oid = node->get_instance_id();
 	// See if the path is cached.
-	PathSentCache *psc = path_send_cache.getptr(for_path);
+	PathSentCache *psc = path_send_cache.getptr(oid);
 	if (!psc) {
 		// Path is not cached, create.
-		path_send_cache[for_path] = PathSentCache();
-		psc = path_send_cache.getptr(for_path);
+		path_send_cache[oid] = PathSentCache();
+		psc = path_send_cache.getptr(oid);
 		psc->id = last_send_cache_id++;
 	}
 	return psc->id;
@@ -189,11 +194,16 @@ int SceneCacheInterface::make_object_cache(Object *p_obj) {
 bool SceneCacheInterface::send_object_cache(Object *p_obj, int p_peer_id, int &r_id) {
 	Node *node = Object::cast_to<Node>(p_obj);
 	ERR_FAIL_NULL_V(node, false);
-
-	r_id = make_object_cache(p_obj);
-	ERR_FAIL_COND_V(r_id < 0, false);
-	NodePath for_path = multiplayer->get_root_path().rel_path_to(node->get_path());
-	PathSentCache *psc = path_send_cache.getptr(for_path);
+	const ObjectID oid = node->get_instance_id();
+	// See if the path is cached.
+	PathSentCache *psc = path_send_cache.getptr(oid);
+	if (!psc) {
+		// Path is not cached, create.
+		path_send_cache[oid] = PathSentCache();
+		psc = path_send_cache.getptr(oid);
+		psc->id = last_send_cache_id++;
+	}
+	r_id = psc->id;
 
 	bool has_all_peers = true;
 	List<int> peers_to_add; // If one is missing, take note to add it.
@@ -225,7 +235,7 @@ bool SceneCacheInterface::send_object_cache(Object *p_obj, int p_peer_id, int &r
 	}
 
 	if (peers_to_add.size()) {
-		_send_confirm_path(node, for_path, psc, peers_to_add);
+		_send_confirm_path(node, psc, peers_to_add);
 	}
 
 	return has_all_peers;

modules/multiplayer/scene_cache_interface.h

@@ -58,12 +58,12 @@ class SceneCacheInterface : public RefCounted {
 		HashMap<int, NodeInfo> nodes;
 	};
 
-	HashMap<NodePath, PathSentCache> path_send_cache;
+	HashMap<ObjectID, PathSentCache> path_send_cache;
 	HashMap<int, PathGetCache> path_get_cache;
 	int last_send_cache_id = 1;
 
 protected:
-	Error _send_confirm_path(Node *p_node, NodePath p_path, PathSentCache *psc, const List<int> &p_peers);
+	Error _send_confirm_path(Node *p_node, PathSentCache *psc, const List<int> &p_peers);
 
 public:
 	void clear();
@@ -75,7 +75,7 @@ class SceneCacheInterface : public RefCounted {
 	bool send_object_cache(Object *p_obj, int p_target, int &p_id);
 	int make_object_cache(Object *p_obj);
 	Object *get_cached_object(int p_from, uint32_t p_cache_id);
-	bool is_cache_confirmed(NodePath p_path, int p_peer);
+	bool is_cache_confirmed(Node *p_path, int p_peer);
 
 	SceneCacheInterface(SceneMultiplayer *p_multiplayer) { multiplayer = p_multiplayer; }
 };

modules/multiplayer/scene_multiplayer.cpp

@@ -680,12 +680,16 @@ void SceneMultiplayer::_bind_methods() {
 
 SceneMultiplayer::SceneMultiplayer() {
 	relay_buffer.instantiate();
-	replicator = Ref<SceneReplicationInterface>(memnew(SceneReplicationInterface(this)));
-	rpc = Ref<SceneRPCInterface>(memnew(SceneRPCInterface(this)));
 	cache = Ref<SceneCacheInterface>(memnew(SceneCacheInterface(this)));
+	replicator = Ref<SceneReplicationInterface>(memnew(SceneReplicationInterface(this, cache.ptr())));
+	rpc = Ref<SceneRPCInterface>(memnew(SceneRPCInterface(this, cache.ptr(), replicator.ptr())));
 	set_multiplayer_peer(Ref<OfflineMultiplayerPeer>(memnew(OfflineMultiplayerPeer)));
 }
 
 SceneMultiplayer::~SceneMultiplayer() {
 	clear();
+	// Ensure unref in reverse order for safety (we shouldn't use those pointers in the deconstructors anyway).
+	rpc.unref();
+	replicator.unref();
+	cache.unref();
 }

modules/multiplayer/scene_multiplayer.h

@@ -201,9 +201,6 @@ class SceneMultiplayer : public MultiplayerAPI {
 	void set_max_delta_packet_size(int p_size);
 	int get_max_delta_packet_size() const;
 
-	Ref<SceneCacheInterface> get_path_cache() { return cache; }
-	Ref<SceneReplicationInterface> get_replicator() { return replicator; }
-
 	SceneMultiplayer();
 	~SceneMultiplayer();
 };

modules/multiplayer/scene_replication_interface.cpp

@@ -89,6 +89,10 @@ void SceneReplicationInterface::_free_remotes(const PeerInfo &p_info) {
 	}
 }
 
+bool SceneReplicationInterface::_has_authority(const Node *p_node) {
+	return multiplayer->has_multiplayer_peer() && p_node->get_multiplayer_authority() == multiplayer->get_unique_id();
+}
+
 void SceneReplicationInterface::on_peer_change(int p_id, bool p_connected) {
 	if (p_connected) {
 		peers_info[p_id] = PeerInfo();
@@ -184,7 +188,7 @@ void SceneReplicationInterface::_node_ready(const ObjectID &p_oid) {
 		ERR_CONTINUE(!spawner);
 
 		spawned_nodes.insert(oid);
-		if (multiplayer->has_multiplayer_peer() && spawner->is_multiplayer_authority()) {
+		if (_has_authority(spawner)) {
 			if (tobj.net_id == 0) {
 				tobj.net_id = ++last_net_id;
 			}
@@ -248,9 +252,9 @@ Error SceneReplicationInterface::on_replication_start(Object *p_obj, Variant p_c
 
 		// Try to apply spawn state (before ready).
 		if (pending_buffer_size > 0) {
-			ERR_FAIL_COND_V(!node || sync->get_replication_config().is_null(), ERR_UNCONFIGURED);
+			ERR_FAIL_COND_V(!node || !sync->get_replication_config_ptr(), ERR_UNCONFIGURED);
 			int consumed = 0;
-			const List<NodePath> props = sync->get_replication_config()->get_spawn_properties();
+			const List<NodePath> props = sync->get_replication_config_ptr()->get_spawn_properties();
 			Vector<Variant> vars;
 			vars.resize(props.size());
 			Error err = MultiplayerAPI::decode_and_decompress_variants(vars, pending_buffer, pending_buffer_size, consumed);
@@ -342,7 +346,7 @@ bool SceneReplicationInterface::is_rpc_visible(const ObjectID &p_oid, int p_peer
 
 Error SceneReplicationInterface::_update_sync_visibility(int p_peer, MultiplayerSynchronizer *p_sync) {
 	ERR_FAIL_NULL_V(p_sync, ERR_BUG);
-	if (!multiplayer->has_multiplayer_peer() || !p_sync->is_multiplayer_authority() || p_peer == multiplayer->get_unique_id()) {
+	if (!_has_authority(p_sync) || p_peer == multiplayer->get_unique_id()) {
 		return OK;
 	}
 
@@ -383,14 +387,16 @@ Error SceneReplicationInterface::_update_spawn_visibility(int p_peer, const Obje
 	ERR_FAIL_NULL_V(tnode, ERR_BUG);
 	MultiplayerSpawner *spawner = get_id_as<MultiplayerSpawner>(tnode->spawner);
 	Node *node = get_id_as<Node>(p_oid);
-	ERR_FAIL_COND_V(!node || !spawner || !spawner->is_multiplayer_authority(), ERR_BUG);
+	ERR_FAIL_NULL_V(node, ERR_BUG);
+	ERR_FAIL_NULL_V(spawner, ERR_BUG);
+	ERR_FAIL_COND_V(!_has_authority(spawner), ERR_BUG);
 	ERR_FAIL_COND_V(!tracked_nodes.has(p_oid), ERR_BUG);
 	const HashSet<ObjectID> synchronizers = tracked_nodes[p_oid].synchronizers;
 	bool is_visible = true;
 	for (const ObjectID &sid : synchronizers) {
 		MultiplayerSynchronizer *sync = get_id_as<MultiplayerSynchronizer>(sid);
 		ERR_CONTINUE(!sync);
-		if (!sync->is_multiplayer_authority()) {
+		if (!_has_authority(sync)) {
 			continue;
 		}
 		// Spawn visibility is composed using OR when multiple synchronizers are present.
@@ -435,7 +441,7 @@ Error SceneReplicationInterface::_update_spawn_visibility(int p_peer, const Obje
 		for (int pid : to_spawn) {
 			ERR_CONTINUE(!peers_info.has(pid));
 			int path_id;
-			multiplayer->get_path_cache()->send_object_cache(spawner, pid, path_id);
+			multiplayer_cache->send_object_cache(spawner, pid, path_id);
 			_send_raw(packet_cache.ptr(), len, pid, true);
 			peers_info[pid].spawn_nodes.insert(p_oid);
 		}
@@ -454,9 +460,9 @@ Error SceneReplicationInterface::_update_spawn_visibility(int p_peer, const Obje
 
 Error SceneReplicationInterface::_send_raw(const uint8_t *p_buffer, int p_size, int p_peer, bool p_reliable) {
 	ERR_FAIL_COND_V(!p_buffer || p_size < 1, ERR_INVALID_PARAMETER);
-	ERR_FAIL_COND_V(!multiplayer->has_multiplayer_peer(), ERR_UNCONFIGURED);
 
 	Ref<MultiplayerPeer> peer = multiplayer->get_multiplayer_peer();
+	ERR_FAIL_COND_V(peer.is_null(), ERR_UNCONFIGURED);
 	peer->set_transfer_channel(0);
 	peer->set_transfer_mode(p_reliable ? MultiplayerPeer::TRANSFER_MODE_RELIABLE : MultiplayerPeer::TRANSFER_MODE_UNRELIABLE);
 	return multiplayer->send_command(p_peer, p_buffer, p_size);
@@ -488,12 +494,12 @@ Error SceneReplicationInterface::_make_spawn_packet(Node *p_node, MultiplayerSpa
 	const HashSet<ObjectID> synchronizers = tnode->synchronizers;
 	for (const ObjectID &sid : synchronizers) {
 		MultiplayerSynchronizer *sync = get_id_as<MultiplayerSynchronizer>(sid);
-		if (!sync->is_multiplayer_authority()) {
+		if (!_has_authority(sync)) {
 			continue;
 		}
 		ERR_CONTINUE(!sync);
-		ERR_FAIL_COND_V(sync->get_replication_config().is_null(), ERR_BUG);
-		for (const NodePath &prop : sync->get_replication_config()->get_spawn_properties()) {
+		ERR_FAIL_NULL_V(sync->get_replication_config_ptr(), ERR_BUG);
+		for (const NodePath &prop : sync->get_replication_config_ptr()->get_spawn_properties()) {
 			state_props.push_back(prop);
 		}
 		// Ensure the synchronizer has an ID.
@@ -513,7 +519,7 @@ Error SceneReplicationInterface::_make_spawn_packet(Node *p_node, MultiplayerSpa
 	}
 
 	// Encode scene ID, path ID, net ID, node name.
-	int path_id = multiplayer->get_path_cache()->make_object_cache(p_spawner);
+	int path_id = multiplayer_cache->make_object_cache(p_spawner);
 	CharString cname = p_node->get_name().operator String().utf8();
 	int nlen = encode_cstring(cname.get_data(), nullptr);
 	MAKE_ROOM(1 + 1 + 4 + 4 + 4 + 4 * sync_ids.size() + 4 + nlen + (is_custom ? 4 + spawn_arg_size : 0) + state_size);
@@ -567,7 +573,7 @@ Error SceneReplicationInterface::on_spawn_receive(int p_from, const uint8_t *p_b
 	ofs += 1;
 	uint32_t node_target = decode_uint32(&p_buffer[ofs]);
 	ofs += 4;
-	MultiplayerSpawner *spawner = Object::cast_to<MultiplayerSpawner>(multiplayer->get_path_cache()->get_cached_object(p_from, node_target));
+	MultiplayerSpawner *spawner = Object::cast_to<MultiplayerSpawner>(multiplayer_cache->get_cached_object(p_from, node_target));
 	ERR_FAIL_NULL_V(spawner, ERR_DOES_NOT_EXIST);
 	ERR_FAIL_COND_V(p_from != spawner->get_multiplayer_authority(), ERR_UNAUTHORIZED);
 
@@ -678,7 +684,7 @@ bool SceneReplicationInterface::_verify_synchronizer(int p_peer, MultiplayerSync
 	r_net_id = p_sync->get_net_id();
 	if (r_net_id == 0 || (r_net_id & 0x80000000)) {
 		int path_id = 0;
-		bool verified = multiplayer->get_path_cache()->send_object_cache(p_sync, p_peer, path_id);
+		bool verified = multiplayer_cache->send_object_cache(p_sync, p_peer, path_id);
 		ERR_FAIL_COND_V_MSG(path_id < 0, false, "This should never happen!");
 		if (r_net_id == 0) {
 			// First time path based ID.
@@ -693,7 +699,7 @@ bool SceneReplicationInterface::_verify_synchronizer(int p_peer, MultiplayerSync
 MultiplayerSynchronizer *SceneReplicationInterface::_find_synchronizer(int p_peer, uint32_t p_net_id) {
 	MultiplayerSynchronizer *sync = nullptr;
 	if (p_net_id & 0x80000000) {
-		sync = Object::cast_to<MultiplayerSynchronizer>(multiplayer->get_path_cache()->get_cached_object(p_peer, p_net_id & 0x7FFFFFFF));
+		sync = Object::cast_to<MultiplayerSynchronizer>(multiplayer_cache->get_cached_object(p_peer, p_net_id & 0x7FFFFFFF));
 	} else if (peers_info[p_peer].recv_sync_ids.has(p_net_id)) {
 		const ObjectID &sid = peers_info[p_peer].recv_sync_ids[p_net_id];
 		sync = get_id_as<MultiplayerSynchronizer>(sid);
@@ -708,7 +714,7 @@ void SceneReplicationInterface::_send_delta(int p_peer, const HashSet<ObjectID>
 	int ofs = 1;
 	for (const ObjectID &oid : p_synchronizers) {
 		MultiplayerSynchronizer *sync = get_id_as<MultiplayerSynchronizer>(oid);
-		ERR_CONTINUE(!sync || !sync->get_replication_config().is_valid() || !sync->is_multiplayer_authority());
+		ERR_CONTINUE(!sync || !sync->get_replication_config_ptr() || !_has_authority(sync));
 		uint32_t net_id;
 		if (!_verify_synchronizer(p_peer, sync, net_id)) {
 			continue;
@@ -803,7 +809,7 @@ void SceneReplicationInterface::_send_sync(int p_peer, const HashSet<ObjectID> p
 	// This is a lazy implementation, we could optimize much more here with by grouping by replication config.
 	for (const ObjectID &oid : p_synchronizers) {
 		MultiplayerSynchronizer *sync = get_id_as<MultiplayerSynchronizer>(oid);
-		ERR_CONTINUE(!sync || !sync->get_replication_config().is_valid() || !sync->is_multiplayer_authority());
+		ERR_CONTINUE(!sync || !sync->get_replication_config_ptr() || !_has_authority(sync));
 		if (!sync->update_outbound_sync_time(p_usec)) {
 			continue; // nothing to sync.
 		}
@@ -818,7 +824,7 @@ void SceneReplicationInterface::_send_sync(int p_peer, const HashSet<ObjectID> p
 		int size;
 		Vector<Variant> vars;
 		Vector<const Variant *> varp;
-		const List<NodePath> props = sync->get_replication_config()->get_sync_properties();
+		const List<NodePath> props = sync->get_replication_config_ptr()->get_sync_properties();
 		Error err = MultiplayerSynchronizer::get_state(props, node, vars, varp);
 		ERR_CONTINUE_MSG(err != OK, "Unable to retrieve sync state.");
 		err = MultiplayerAPI::encode_and_compress_variants(varp.ptrw(), varp.size(), nullptr, size);
@@ -877,7 +883,7 @@ Error SceneReplicationInterface::on_sync_receive(int p_from, const uint8_t *p_bu
 			ofs += size;
 			continue;
 		}
-		const List<NodePath> props = sync->get_replication_config()->get_sync_properties();
+		const List<NodePath> props = sync->get_replication_config_ptr()->get_sync_properties();
 		Vector<Variant> vars;
 		vars.resize(props.size());
 		int consumed;