Merge pull request #50566 from reduz/optimize-stringname-usage

Optimize StringName usage

5 files changed, 50 insertions, 26 deletions

diff --git a/core/input/input.cpp b/core/input/input.cpp
index a712394b35..f57985831a 100644
--- a/core/input/input.cpp
+++ b/core/input/input.cpp
@@ -438,7 +438,7 @@ void Input::joy_connection_changed(int p_idx, bool p_connected, String p_name, S
 	}
 	joy_names[p_idx] = js;
 
-	emit_signal("joy_connection_changed", p_idx, p_connected);
+	emit_signal(SNAME("joy_connection_changed"), p_idx, p_connected);
 }
 
 Vector3 Input::get_gravity() const {
diff --git a/core/io/multiplayer_api.cpp b/core/io/multiplayer_api.cpp
index 9f92388196..0745757d48 100644
--- a/core/io/multiplayer_api.cpp
+++ b/core/io/multiplayer_api.cpp
@@ -941,7 +941,7 @@ void MultiplayerAPI::_send_rpc(Node *p_from, int p_to, uint16_t p_rpc_id, const
 void MultiplayerAPI::_add_peer(int p_id) {
 	connected_peers.insert(p_id);
 	path_get_cache.insert(p_id, PathGetCache());
-	emit_signal("network_peer_connected", p_id);
+	emit_signal(SNAME("network_peer_connected"), p_id);
 }
 
 void MultiplayerAPI::_del_peer(int p_id) {
@@ -956,19 +956,19 @@ void MultiplayerAPI::_del_peer(int p_id) {
 		PathSentCache *psc = path_send_cache.getptr(E->get());
 		psc->confirmed_peers.erase(p_id);
 	}
-	emit_signal("network_peer_disconnected", p_id);
+	emit_signal(SNAME("network_peer_disconnected"), p_id);
 }
 
 void MultiplayerAPI::_connected_to_server() {
-	emit_signal("connected_to_server");
+	emit_signal(SNAME("connected_to_server"));
 }
 
 void MultiplayerAPI::_connection_failed() {
-	emit_signal("connection_failed");
+	emit_signal(SNAME("connection_failed"));
 }
 
 void MultiplayerAPI::_server_disconnected() {
-	emit_signal("server_disconnected");
+	emit_signal(SNAME("server_disconnected"));
 }
 
 void MultiplayerAPI::rpcp(Node *p_node, int p_peer_id, bool p_unreliable, const StringName &p_method, const Variant **p_arg, int p_argcount) {
@@ -1059,7 +1059,7 @@ void MultiplayerAPI::_process_raw(int p_from, const uint8_t *p_packet, int p_pac
 		uint8_t *w = out.ptrw();
 		memcpy(&w[0], &p_packet[1], len);
 	}
-	emit_signal("network_peer_packet", p_from, out);
+	emit_signal(SNAME("network_peer_packet"), p_from, out);
 }
 
 int MultiplayerAPI::get_network_unique_id() const {
diff --git a/core/object/undo_redo.cpp b/core/object/undo_redo.cpp
index 96c96c1efb..0b3d93dc3a 100644
--- a/core/object/undo_redo.cpp
+++ b/core/object/undo_redo.cpp
@@ -65,7 +65,7 @@ bool UndoRedo::_redo(bool p_execute) {
 		_process_operation_list(actions.write[current_action].do_ops.front());
 	}
 	version++;
-	emit_signal("version_changed");
+	emit_signal(SNAME("version_changed"));
 
 	return true;
 }
@@ -352,7 +352,7 @@ bool UndoRedo::undo() {
 	_process_operation_list(actions.write[current_action].undo_ops.front());
 	current_action--;
 	version--;
-	emit_signal("version_changed");
+	emit_signal(SNAME("version_changed"));
 
 	return true;
 }
@@ -385,7 +385,7 @@ void UndoRedo::clear_history(bool p_increase_version) {
 
 	if (p_increase_version) {
 		version++;
-		emit_signal("version_changed");
+		emit_signal(SNAME("version_changed"));
 	}
 }
 
diff --git a/core/string/string_name.cpp b/core/string/string_name.cpp
index 14b87072bb..44ebafc643 100644
--- a/core/string/string_name.cpp
+++ b/core/string/string_name.cpp
@@ -41,8 +41,8 @@ StaticCString StaticCString::create(const char *p_ptr) {
 
 StringName::_Data *StringName::_table[STRING_TABLE_LEN];
 
-StringName _scs_create(const char *p_chr) {
-	return (p_chr[0] ? StringName(StaticCString::create(p_chr)) : StringName());
+StringName _scs_create(const char *p_chr, bool p_static) {
+	return (p_chr[0] ? StringName(StaticCString::create(p_chr), p_static) : StringName());
 }
 
 bool StringName::configured = false;
@@ -64,7 +64,7 @@ void StringName::cleanup() {
 		while (_table[i]) {
 			_Data *d = _table[i];
 			lost_strings++;
-			if (OS::get_singleton()->is_stdout_verbose()) {
+			if (d->static_count.get() != d->refcount.get() && OS::get_singleton()->is_stdout_verbose()) {
 				if (d->cname) {
 					print_line("Orphan StringName: " + String(d->cname));
 				} else {
@@ -79,6 +79,7 @@ void StringName::cleanup() {
 	if (lost_strings) {
 		print_verbose("StringName: " + itos(lost_strings) + " unclaimed string names at exit.");
 	}
+	configured = false;
 }
 
 void StringName::unref() {
@@ -87,6 +88,13 @@ void StringName::unref() {
 	if (_data && _data->refcount.unref()) {
 		MutexLock lock(mutex);
 
+		if (_data->static_count.get() > 0) {
+			if (_data->cname) {
+				ERR_PRINT("BUG: Unreferenced static string to 0: " + String(_data->cname));
+			} else {
+				ERR_PRINT("BUG: Unreferenced static string to 0: " + String(_data->name));
+			}
+		}
 		if (_data->prev) {
 			_data->prev->next = _data->next;
 		} else {
@@ -153,7 +161,7 @@ StringName::StringName(const StringName &p_name) {
 	}
 }
 
-StringName::StringName(const char *p_name) {
+StringName::StringName(const char *p_name, bool p_static) {
 	_data = nullptr;
 
 	ERR_FAIL_COND(!configured);
@@ -181,6 +189,9 @@ StringName::StringName(const char *p_name) {
 	if (_data) {
 		if (_data->refcount.ref()) {
 			// exists
+			if (p_static) {
+				_data->static_count.increment();
+			}
 			return;
 		}
 	}
@@ -188,6 +199,7 @@ StringName::StringName(const char *p_name) {
 	_data = memnew(_Data);
 	_data->name = p_name;
 	_data->refcount.init();
+	_data->static_count.set(p_static ? 1 : 0);
 	_data->hash = hash;
 	_data->idx = idx;
 	_data->cname = nullptr;
@@ -199,7 +211,7 @@ StringName::StringName(const char *p_name) {
 	_table[idx] = _data;
 }
 
-StringName::StringName(const StaticCString &p_static_string) {
+StringName::StringName(const StaticCString &p_static_string, bool p_static) {
 	_data = nullptr;
 
 	ERR_FAIL_COND(!configured);
@@ -225,6 +237,9 @@ StringName::StringName(const StaticCString &p_static_string) {
 	if (_data) {
 		if (_data->refcount.ref()) {
 			// exists
+			if (p_static) {
+				_data->static_count.increment();
+			}
 			return;
 		}
 	}
@@ -232,6 +247,7 @@ StringName::StringName(const StaticCString &p_static_string) {
 	_data = memnew(_Data);
 
 	_data->refcount.init();
+	_data->static_count.set(p_static ? 1 : 0);
 	_data->hash = hash;
 	_data->idx = idx;
 	_data->cname = p_static_string.ptr;
@@ -243,7 +259,7 @@ StringName::StringName(const StaticCString &p_static_string) {
 	_table[idx] = _data;
 }
 
-StringName::StringName(const String &p_name) {
+StringName::StringName(const String &p_name, bool p_static) {
 	_data = nullptr;
 
 	ERR_FAIL_COND(!configured);
@@ -269,6 +285,9 @@ StringName::StringName(const String &p_name) {
 	if (_data) {
 		if (_data->refcount.ref()) {
 			// exists
+			if (p_static) {
+				_data->static_count.increment();
+			}
 			return;
 		}
 	}
@@ -276,6 +295,7 @@ StringName::StringName(const String &p_name) {
 	_data = memnew(_Data);
 	_data->name = p_name;
 	_data->refcount.init();
+	_data->static_count.set(p_static ? 1 : 0);
 	_data->hash = hash;
 	_data->idx = idx;
 	_data->cname = nullptr;
@@ -374,10 +394,6 @@ StringName StringName::search(const String &p_name) {
 	return StringName(); //does not exist
 }
 
-StringName::~StringName() {
-	unref();
-}
-
 bool operator==(const String &p_name, const StringName &p_string_name) {
 	return p_name == p_string_name.operator String();
 }
diff --git a/core/string/string_name.h b/core/string/string_name.h
index 44d0ea14fa..ead321c5b0 100644
--- a/core/string/string_name.h
+++ b/core/string/string_name.h
@@ -44,13 +44,15 @@ struct StaticCString {
 
 class StringName {
 	enum {
-		STRING_TABLE_BITS = 12,
+		STRING_TABLE_BITS = 16,
 		STRING_TABLE_LEN = 1 << STRING_TABLE_BITS,
 		STRING_TABLE_MASK = STRING_TABLE_LEN - 1
 	};
 
 	struct _Data {
 		SafeRefCount refcount;
+		SafeNumeric<uint32_t> static_count;
+		SafeRefCount static_refcount;
 		const char *cname = nullptr;
 		String name;
 
@@ -146,12 +148,16 @@ public:
 	};
 
 	void operator=(const StringName &p_name);
-	StringName(const char *p_name);
+	StringName(const char *p_name, bool p_static = false);
 	StringName(const StringName &p_name);
-	StringName(const String &p_name);
-	StringName(const StaticCString &p_static_string);
+	StringName(const String &p_name, bool p_static = false);
+	StringName(const StaticCString &p_static_string, bool p_static = false);
 	StringName() {}
-	~StringName();
+	_FORCE_INLINE_ ~StringName() {
+		if (likely(configured) && _data) { //only free if configured
+			unref();
+		}
+	}
 };
 
 bool operator==(const String &p_name, const StringName &p_string_name);
@@ -159,6 +165,8 @@ bool operator!=(const String &p_name, const StringName &p_string_name);
 bool operator==(const char *p_name, const StringName &p_string_name);
 bool operator!=(const char *p_name, const StringName &p_string_name);
 
-StringName _scs_create(const char *p_chr);
+StringName _scs_create(const char *p_chr, bool p_static = false);
+
+#define SNAME(m_arg) ([]() { static StringName sname = _scs_create(m_arg, true); return sname; })()
 
 #endif // STRING_NAME_H