From 311a27281f8f04335f079eef506798903296192f Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Wed, 27 Sep 2023 01:47:23 +0200 Subject: [PATCH] [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 3e3118b1cd4..04de3dfb7f3 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 a61e5056898..e799abeb48b 100644 --- a/modules/multiplayer/scene_multiplayer.h +++ b/modules/multiplayer/scene_multiplayer.h @@ -201,9 +201,6 @@ public: 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 0404bc982df..fc363ee0f52 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 4cc2f20ffae..3b3ec6a9efb 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 @@ private: // 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 @@ public: 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 5301815c1e2..1463598ddc6 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 b40169a63b7..5c9b66d5f55 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 @@ private: }; SceneMultiplayer *multiplayer = nullptr; + SceneCacheInterface *multiplayer_cache = nullptr; + SceneReplicationInterface *multiplayer_replicator = nullptr; + Vector packet_cache; HashMap rpc_cache; @@ -99,7 +104,11 @@ public: 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