From 61cf7d180cc6ac1be1f9cb1c7369b2cae5d99b25 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Tue, 26 Sep 2023 20:27:53 +0200 Subject: [PATCH 1/4] [MP] Optimize multiplayer NodePath caching Only use paths during network transfer. Use ObjectID instead of NodePaths for storing the Node <-> NetID relations locally. --- modules/multiplayer/scene_cache_interface.cpp | 52 +++++++++++-------- modules/multiplayer/scene_cache_interface.h | 6 +-- modules/multiplayer/scene_rpc_interface.cpp | 5 +- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/modules/multiplayer/scene_cache_interface.cpp b/modules/multiplayer/scene_cache_interface.cpp index 8d102ca98126..56cd0bec18b6 100644 --- a/modules/multiplayer/scene_cache_interface.cpp +++ b/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 &E : path_send_cache) { - PathSentCache *psc = path_send_cache.getptr(E.key); - psc->confirmed_peers.erase(p_id); + for (KeyValue &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,19 +105,24 @@ 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::Iterator E = psc->confirmed_peers.find(p_from); @@ -126,9 +130,9 @@ void SceneCacheInterface::process_confirm_path(int p_from, const uint8_t *p_pack E->value = true; } -Error SceneCacheInterface::_send_confirm_path(Node *p_node, NodePath p_path, PathSentCache *psc, const List &p_peers) { +Error SceneCacheInterface::_send_confirm_path(Node *p_node, PathSentCache *psc, const List &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::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(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(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 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; diff --git a/modules/multiplayer/scene_cache_interface.h b/modules/multiplayer/scene_cache_interface.h index 7a7304fde8b5..e63beb5f84f8 100644 --- a/modules/multiplayer/scene_cache_interface.h +++ b/modules/multiplayer/scene_cache_interface.h @@ -58,12 +58,12 @@ class SceneCacheInterface : public RefCounted { HashMap nodes; }; - HashMap path_send_cache; + HashMap path_send_cache; HashMap path_get_cache; int last_send_cache_id = 1; protected: - Error _send_confirm_path(Node *p_node, NodePath p_path, PathSentCache *psc, const List &p_peers); + Error _send_confirm_path(Node *p_node, PathSentCache *psc, const List &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; } }; diff --git a/modules/multiplayer/scene_rpc_interface.cpp b/modules/multiplayer/scene_rpc_interface.cpp index 48e1d13f9cfc..bf0a261bd884 100644 --- a/modules/multiplayer/scene_rpc_interface.cpp +++ b/modules/multiplayer/scene_rpc_interface.cpp @@ -443,15 +443,14 @@ void SceneRPCInterface::_send_rpc(Node *p_node, int p_to, uint16_t p_rpc_id, con // Not all verified path, so send one by one. // Append path at the end, since we will need it for some packets. - NodePath from_path = multiplayer->get_root_path().rel_path_to(p_node->get_path()); - CharString pname = String(from_path).utf8(); + CharString pname = String(multiplayer->get_root_path().rel_path_to(p_node->get_path())).utf8(); int path_len = encode_cstring(pname.get_data(), nullptr); MAKE_ROOM(ofs + path_len); encode_cstring(pname.get_data(), &(packet_cache.write[ofs])); // Not all verified path, so check which needs the longer packet. for (const int P : targets) { - bool confirmed = multiplayer->get_path_cache()->is_cache_confirmed(from_path, P); + bool confirmed = multiplayer->get_path_cache()->is_cache_confirmed(p_node, P); if (confirmed) { // This one confirmed path, so use id. encode_uint32(psc_id, &(packet_cache.write[1])); From 9ce423914ec4a392e42dbf894dc7929befcc7e1d Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Wed, 27 Sep 2023 01:12:58 +0200 Subject: [PATCH 2/4] [MP] Optimize internal authority checks We already know which MultiplayerAPI a certain Node uses, so we don't need to retrieve it via SceneTree every time. --- .../scene_replication_interface.cpp | 22 +++++++++----- .../multiplayer/scene_replication_interface.h | 1 + modules/multiplayer/scene_rpc_interface.cpp | 30 ++++++++----------- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/modules/multiplayer/scene_replication_interface.cpp b/modules/multiplayer/scene_replication_interface.cpp index e350f2f68bec..0404bc982df1 100644 --- a/modules/multiplayer/scene_replication_interface.cpp +++ b/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; } @@ -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(tnode->spawner); Node *node = get_id_as(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 synchronizers = tracked_nodes[p_oid].synchronizers; bool is_visible = true; for (const ObjectID &sid : synchronizers) { MultiplayerSynchronizer *sync = get_id_as(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. @@ -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 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,7 +494,7 @@ Error SceneReplicationInterface::_make_spawn_packet(Node *p_node, MultiplayerSpa const HashSet synchronizers = tnode->synchronizers; for (const ObjectID &sid : synchronizers) { MultiplayerSynchronizer *sync = get_id_as(sid); - if (!sync->is_multiplayer_authority()) { + if (!_has_authority(sync)) { continue; } ERR_CONTINUE(!sync); @@ -708,7 +714,7 @@ void SceneReplicationInterface::_send_delta(int p_peer, const HashSet int ofs = 1; for (const ObjectID &oid : p_synchronizers) { MultiplayerSynchronizer *sync = get_id_as(oid); - ERR_CONTINUE(!sync || !sync->get_replication_config().is_valid() || !sync->is_multiplayer_authority()); + ERR_CONTINUE(!sync || !sync->get_replication_config().is_valid() || !_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 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(oid); - ERR_CONTINUE(!sync || !sync->get_replication_config().is_valid() || !sync->is_multiplayer_authority()); + ERR_CONTINUE(!sync || !sync->get_replication_config().is_valid() || !_has_authority(sync)); if (!sync->update_outbound_sync_time(p_usec)) { continue; // nothing to sync. } diff --git a/modules/multiplayer/scene_replication_interface.h b/modules/multiplayer/scene_replication_interface.h index 267d329ca7e9..4cc2f20ffaed 100644 --- a/modules/multiplayer/scene_replication_interface.h +++ b/modules/multiplayer/scene_replication_interface.h @@ -95,6 +95,7 @@ class SceneReplicationInterface : public RefCounted { void _untrack(const ObjectID &p_id); void _node_ready(const ObjectID &p_oid); + bool _has_authority(const Node *p_node); bool _verify_synchronizer(int p_peer, MultiplayerSynchronizer *p_sync, uint32_t &r_net_id); MultiplayerSynchronizer *_find_synchronizer(int p_peer, uint32_t p_net_ida); diff --git a/modules/multiplayer/scene_rpc_interface.cpp b/modules/multiplayer/scene_rpc_interface.cpp index bf0a261bd884..5301815c1e2c 100644 --- a/modules/multiplayer/scene_rpc_interface.cpp +++ b/modules/multiplayer/scene_rpc_interface.cpp @@ -116,22 +116,6 @@ const SceneRPCInterface::RPCConfigCache &SceneRPCInterface::_get_node_config(con return rpc_cache[oid]; } -_FORCE_INLINE_ bool _can_call_mode(Node *p_node, MultiplayerAPI::RPCMode mode, int p_remote_id) { - switch (mode) { - case MultiplayerAPI::RPC_MODE_DISABLED: { - return false; - } break; - case MultiplayerAPI::RPC_MODE_ANY_PEER: { - return true; - } break; - case MultiplayerAPI::RPC_MODE_AUTHORITY: { - return !p_node->is_multiplayer_authority() && p_remote_id == p_node->get_multiplayer_authority(); - } break; - } - - return false; -} - String SceneRPCInterface::get_rpc_md5(const Object *p_obj) { const Node *node = Object::cast_to(p_obj); ERR_FAIL_NULL_V(node, ""); @@ -252,7 +236,19 @@ void SceneRPCInterface::_process_rpc(Node *p_node, const uint16_t p_rpc_method_i ERR_FAIL_COND(!cache_config.configs.has(p_rpc_method_id)); const RPCConfig &config = cache_config.configs[p_rpc_method_id]; - bool can_call = _can_call_mode(p_node, config.rpc_mode, p_from); + bool can_call = false; + switch (config.rpc_mode) { + case MultiplayerAPI::RPC_MODE_DISABLED: { + can_call = false; + } break; + case MultiplayerAPI::RPC_MODE_ANY_PEER: { + can_call = true; + } break; + case MultiplayerAPI::RPC_MODE_AUTHORITY: { + can_call = p_from == p_node->get_multiplayer_authority(); + } break; + } + ERR_FAIL_COND_MSG(!can_call, "RPC '" + String(config.name) + "' is not allowed on node " + p_node->get_path() + " from: " + itos(p_from) + ". Mode is " + itos((int)config.rpc_mode) + ", authority is " + itos(p_node->get_multiplayer_authority()) + "."); int argc = 0; From 311a27281f8f04335f079eef506798903296192f Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Wed, 27 Sep 2023 01:47:23 +0200 Subject: [PATCH 3/4] [MP] Avoid unnecessary internal ref/unrefs Access the various internal components (cache/replicator) via pointer, to avoid unnecessary overhead. --- modules/multiplayer/scene_multiplayer.cpp | 8 ++++++-- modules/multiplayer/scene_multiplayer.h | 3 --- .../multiplayer/scene_replication_interface.cpp | 10 +++++----- modules/multiplayer/scene_replication_interface.h | 5 ++++- modules/multiplayer/scene_rpc_interface.cpp | 15 +++++++-------- modules/multiplayer/scene_rpc_interface.h | 11 ++++++++++- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/modules/multiplayer/scene_multiplayer.cpp b/modules/multiplayer/scene_multiplayer.cpp index 3e3118b1cd44..04de3dfb7f38 100644 --- a/modules/multiplayer/scene_multiplayer.cpp +++ b/modules/multiplayer/scene_multiplayer.cpp @@ -680,12 +680,16 @@ void SceneMultiplayer::_bind_methods() { SceneMultiplayer::SceneMultiplayer() { relay_buffer.instantiate(); - replicator = Ref(memnew(SceneReplicationInterface(this))); - rpc = Ref(memnew(SceneRPCInterface(this))); cache = Ref(memnew(SceneCacheInterface(this))); + replicator = Ref(memnew(SceneReplicationInterface(this, cache.ptr()))); + rpc = Ref(memnew(SceneRPCInterface(this, cache.ptr(), replicator.ptr()))); set_multiplayer_peer(Ref(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(); } diff --git a/modules/multiplayer/scene_multiplayer.h b/modules/multiplayer/scene_multiplayer.h index a61e50568985..e799abeb48b8 100644 --- a/modules/multiplayer/scene_multiplayer.h +++ b/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 get_path_cache() { return cache; } - Ref get_replicator() { return replicator; } - SceneMultiplayer(); ~SceneMultiplayer(); }; diff --git a/modules/multiplayer/scene_replication_interface.cpp b/modules/multiplayer/scene_replication_interface.cpp index 0404bc982df1..fc363ee0f521 100644 --- a/modules/multiplayer/scene_replication_interface.cpp +++ b/modules/multiplayer/scene_replication_interface.cpp @@ -441,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); } @@ -519,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); @@ -573,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(multiplayer->get_path_cache()->get_cached_object(p_from, node_target)); + MultiplayerSpawner *spawner = Object::cast_to(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); @@ -684,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. @@ -699,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(multiplayer->get_path_cache()->get_cached_object(p_peer, p_net_id & 0x7FFFFFFF)); + sync = Object::cast_to(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(sid); diff --git a/modules/multiplayer/scene_replication_interface.h b/modules/multiplayer/scene_replication_interface.h index 4cc2f20ffaed..3b3ec6a9efba 100644 --- a/modules/multiplayer/scene_replication_interface.h +++ b/modules/multiplayer/scene_replication_interface.h @@ -37,6 +37,7 @@ #include "core/object/ref_counted.h" class SceneMultiplayer; +class SceneCacheInterface; class SceneReplicationInterface : public RefCounted { GDCLASS(SceneReplicationInterface, RefCounted); @@ -87,6 +88,7 @@ class SceneReplicationInterface : public RefCounted { // Replicator config. SceneMultiplayer *multiplayer = nullptr; + SceneCacheInterface *multiplayer_cache = nullptr; PackedByteArray packet_cache; int sync_mtu = 1350; // Highly dependent on underlying protocol. int delta_mtu = 65535; @@ -144,8 +146,9 @@ class SceneReplicationInterface : public RefCounted { void set_max_delta_packet_size(int p_size); int get_max_delta_packet_size() const; - SceneReplicationInterface(SceneMultiplayer *p_multiplayer) { + SceneReplicationInterface(SceneMultiplayer *p_multiplayer, SceneCacheInterface *p_cache) { multiplayer = p_multiplayer; + multiplayer_cache = p_cache; } }; diff --git a/modules/multiplayer/scene_rpc_interface.cpp b/modules/multiplayer/scene_rpc_interface.cpp index 5301815c1e2c..1463598ddc61 100644 --- a/modules/multiplayer/scene_rpc_interface.cpp +++ b/modules/multiplayer/scene_rpc_interface.cpp @@ -151,7 +151,7 @@ Node *SceneRPCInterface::_process_get_node(int p_from, const uint8_t *p_packet, return node; } else { // Use cached path. - return Object::cast_to(multiplayer->get_path_cache()->get_cached_object(p_from, p_node_target)); + return Object::cast_to(multiplayer_cache->get_cached_object(p_from, p_node_target)); } } @@ -309,25 +309,24 @@ void SceneRPCInterface::_send_rpc(Node *p_node, int p_to, uint16_t p_rpc_id, con // See if all peers have cached path (if so, call can be fast) while building the RPC target list. HashSet targets; - Ref cache = multiplayer->get_path_cache(); int psc_id = -1; bool has_all_peers = true; const ObjectID oid = p_node->get_instance_id(); if (p_to > 0) { - ERR_FAIL_COND_MSG(!multiplayer->get_replicator()->is_rpc_visible(oid, p_to), "Attempt to call an RPC to a peer that cannot see this node. Peer ID: " + itos(p_to)); + ERR_FAIL_COND_MSG(!multiplayer_replicator->is_rpc_visible(oid, p_to), "Attempt to call an RPC to a peer that cannot see this node. Peer ID: " + itos(p_to)); targets.insert(p_to); - has_all_peers = cache->send_object_cache(p_node, p_to, psc_id); + has_all_peers = multiplayer_cache->send_object_cache(p_node, p_to, psc_id); } else { - bool restricted = !multiplayer->get_replicator()->is_rpc_visible(oid, 0); + bool restricted = !multiplayer_replicator->is_rpc_visible(oid, 0); for (const int &P : multiplayer->get_connected_peers()) { if (p_to < 0 && P == -p_to) { continue; // Excluded peer. } - if (restricted && !multiplayer->get_replicator()->is_rpc_visible(oid, P)) { + if (restricted && !multiplayer_replicator->is_rpc_visible(oid, P)) { continue; // Not visible to this peer. } targets.insert(P); - bool has_peer = cache->send_object_cache(p_node, P, psc_id); + bool has_peer = multiplayer_cache->send_object_cache(p_node, P, psc_id); has_all_peers = has_all_peers && has_peer; } } @@ -446,7 +445,7 @@ void SceneRPCInterface::_send_rpc(Node *p_node, int p_to, uint16_t p_rpc_id, con // Not all verified path, so check which needs the longer packet. for (const int P : targets) { - bool confirmed = multiplayer->get_path_cache()->is_cache_confirmed(p_node, P); + bool confirmed = multiplayer_cache->is_cache_confirmed(p_node, P); if (confirmed) { // This one confirmed path, so use id. encode_uint32(psc_id, &(packet_cache.write[1])); diff --git a/modules/multiplayer/scene_rpc_interface.h b/modules/multiplayer/scene_rpc_interface.h index b40169a63b71..5c9b66d5f55a 100644 --- a/modules/multiplayer/scene_rpc_interface.h +++ b/modules/multiplayer/scene_rpc_interface.h @@ -35,6 +35,8 @@ #include "scene/main/multiplayer_api.h" class SceneMultiplayer; +class SceneCacheInterface; +class SceneReplicationInterface; class Node; class SceneRPCInterface : public RefCounted { @@ -77,6 +79,9 @@ class SceneRPCInterface : public RefCounted { }; SceneMultiplayer *multiplayer = nullptr; + SceneCacheInterface *multiplayer_cache = nullptr; + SceneReplicationInterface *multiplayer_replicator = nullptr; + Vector packet_cache; HashMap rpc_cache; @@ -99,7 +104,11 @@ class SceneRPCInterface : public RefCounted { void process_rpc(int p_from, const uint8_t *p_packet, int p_packet_len); String get_rpc_md5(const Object *p_obj); - SceneRPCInterface(SceneMultiplayer *p_multiplayer) { multiplayer = p_multiplayer; } + SceneRPCInterface(SceneMultiplayer *p_multiplayer, SceneCacheInterface *p_cache, SceneReplicationInterface *p_replicator) { + multiplayer = p_multiplayer; + multiplayer_cache = p_cache; + multiplayer_replicator = p_replicator; + } }; #endif // SCENE_RPC_INTERFACE_H From 5df7577a053375cf1867f6af280b13d9177a2404 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Mon, 2 Oct 2023 10:56:21 +0200 Subject: [PATCH 4/4] [MP] Optimize internal SceneReplicationConfig access Use direct pointer addressing to avoid unnecessary refs/unrefs --- modules/multiplayer/multiplayer_debugger.cpp | 4 ++-- modules/multiplayer/multiplayer_synchronizer.cpp | 4 ++++ modules/multiplayer/multiplayer_synchronizer.h | 1 + .../multiplayer/scene_replication_interface.cpp | 16 ++++++++-------- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/modules/multiplayer/multiplayer_debugger.cpp b/modules/multiplayer/multiplayer_debugger.cpp index 9b05fa884beb..a4d2aed2d61c 100644 --- a/modules/multiplayer/multiplayer_debugger.cpp +++ b/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(); diff --git a/modules/multiplayer/multiplayer_synchronizer.cpp b/modules/multiplayer/multiplayer_synchronizer.cpp index 233f15c3a4a1..21f1f86dbfcd 100644 --- a/modules/multiplayer/multiplayer_synchronizer.cpp +++ b/modules/multiplayer/multiplayer_synchronizer.cpp @@ -441,6 +441,10 @@ List 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); diff --git a/modules/multiplayer/multiplayer_synchronizer.h b/modules/multiplayer/multiplayer_synchronizer.h index 7b77e691d16d..99613de29b5e 100644 --- a/modules/multiplayer/multiplayer_synchronizer.h +++ b/modules/multiplayer/multiplayer_synchronizer.h @@ -118,6 +118,7 @@ class MultiplayerSynchronizer : public Node { List get_delta_state(uint64_t p_cur_usec, uint64_t p_last_usec, uint64_t &r_indexes); List get_delta_properties(uint64_t p_indexes); + SceneReplicationConfig *get_replication_config_ptr() const; MultiplayerSynchronizer(); }; diff --git a/modules/multiplayer/scene_replication_interface.cpp b/modules/multiplayer/scene_replication_interface.cpp index fc363ee0f521..c95e4ff9c99d 100644 --- a/modules/multiplayer/scene_replication_interface.cpp +++ b/modules/multiplayer/scene_replication_interface.cpp @@ -252,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 props = sync->get_replication_config()->get_spawn_properties(); + const List props = sync->get_replication_config_ptr()->get_spawn_properties(); Vector vars; vars.resize(props.size()); Error err = MultiplayerAPI::decode_and_decompress_variants(vars, pending_buffer, pending_buffer_size, consumed); @@ -498,8 +498,8 @@ Error SceneReplicationInterface::_make_spawn_packet(Node *p_node, MultiplayerSpa 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. @@ -714,7 +714,7 @@ void SceneReplicationInterface::_send_delta(int p_peer, const HashSet int ofs = 1; for (const ObjectID &oid : p_synchronizers) { MultiplayerSynchronizer *sync = get_id_as(oid); - ERR_CONTINUE(!sync || !sync->get_replication_config().is_valid() || !_has_authority(sync)); + ERR_CONTINUE(!sync || !sync->get_replication_config_ptr() || !_has_authority(sync)); uint32_t net_id; if (!_verify_synchronizer(p_peer, sync, net_id)) { continue; @@ -809,7 +809,7 @@ void SceneReplicationInterface::_send_sync(int p_peer, const HashSet 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(oid); - ERR_CONTINUE(!sync || !sync->get_replication_config().is_valid() || !_has_authority(sync)); + ERR_CONTINUE(!sync || !sync->get_replication_config_ptr() || !_has_authority(sync)); if (!sync->update_outbound_sync_time(p_usec)) { continue; // nothing to sync. } @@ -824,7 +824,7 @@ void SceneReplicationInterface::_send_sync(int p_peer, const HashSet p int size; Vector vars; Vector varp; - const List props = sync->get_replication_config()->get_sync_properties(); + const List 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); @@ -883,7 +883,7 @@ Error SceneReplicationInterface::on_sync_receive(int p_from, const uint8_t *p_bu ofs += size; continue; } - const List props = sync->get_replication_config()->get_sync_properties(); + const List props = sync->get_replication_config_ptr()->get_sync_properties(); Vector vars; vars.resize(props.size()); int consumed;