From 223ce4fcb958619d0f3c62e79a2b5846240e7ff3 Mon Sep 17 00:00:00 2001 From: Juan Linietsky Date: Thu, 6 Apr 2023 17:54:56 +0200 Subject: [PATCH] Optimize Node::add_child validation Adding 10k nodes is almost twice as fast. --- core/string/ustring.cpp | 79 ++++++++++++++++++++++++++++++--- core/string/ustring.h | 3 +- editor/scene_tree_editor.cpp | 2 +- scene/main/node.cpp | 25 ++++++++++- tests/core/string/test_string.h | 4 +- 5 files changed, 101 insertions(+), 12 deletions(-) diff --git a/core/string/ustring.cpp b/core/string/ustring.cpp index 773445edb695..73b5bc2d5661 100644 --- a/core/string/ustring.cpp +++ b/core/string/ustring.cpp @@ -2593,6 +2593,23 @@ double String::to_float(const wchar_t *p_str, const wchar_t **r_end) { return built_in_strtod(p_str, (wchar_t **)r_end); } +uint32_t String::num_characters(int64_t p_int) { + int r = 1; + if (p_int < 0) { + r += 1; + if (p_int == INT64_MIN) { + p_int = INT64_MAX; + } else { + p_int = -p_int; + } + } + while (p_int >= 10) { + p_int /= 10; + r++; + } + return r; +} + int64_t String::to_int(const char32_t *p_str, int p_len, bool p_clamp) { if (p_len == 0 || !p_str[0]) { return 0; @@ -4561,15 +4578,65 @@ String String::property_name_encode() const { } // Changes made to the set of invalid characters must also be reflected in the String documentation. -const String String::invalid_node_name_characters = ". : @ / \" " UNIQUE_NODE_PREFIX; + +static const char32_t invalid_node_name_characters[] = { '.', ':', '@', '/', '\"', UNIQUE_NODE_PREFIX[0], 0 }; + +String String::get_invalid_node_name_characters() { + // Do not use this function for critical validation. + String r; + const char32_t *c = invalid_node_name_characters; + while (*c) { + if (c != invalid_node_name_characters) { + r += " "; + } + r += String::chr(*c); + c++; + } + return r; +} String String::validate_node_name() const { - Vector chars = String::invalid_node_name_characters.split(" "); - String name = this->replace(chars[0], ""); - for (int i = 1; i < chars.size(); i++) { - name = name.replace(chars[i], ""); + // This is a critical validation in node addition, so it must be optimized. + const char32_t *cn = ptr(); + if (cn == nullptr) { + return String(); } - return name; + bool valid = true; + uint32_t idx = 0; + while (cn[idx]) { + const char32_t *c = invalid_node_name_characters; + while (*c) { + if (cn[idx] == *c) { + valid = false; + break; + } + c++; + } + if (!valid) { + break; + } + idx++; + } + + if (valid) { + return *this; + } + + String validated = *this; + char32_t *nn = validated.ptrw(); + while (nn[idx]) { + const char32_t *c = invalid_node_name_characters; + while (*c) { + if (nn[idx] == *c) { + nn[idx] = '_'; + break; + } + c++; + } + idx++; + } + + return validated; } String String::get_basename() const { diff --git a/core/string/ustring.h b/core/string/ustring.h index 90034b1b07af..e1512cfb2613 100644 --- a/core/string/ustring.h +++ b/core/string/ustring.h @@ -337,6 +337,7 @@ class String { static double to_float(const char *p_str); static double to_float(const wchar_t *p_str, const wchar_t **r_end = nullptr); static double to_float(const char32_t *p_str, const char32_t **r_end = nullptr); + static uint32_t num_characters(int64_t p_int); String capitalize() const; String to_camel_case() const; @@ -432,7 +433,7 @@ class String { String property_name_encode() const; // node functions - static const String invalid_node_name_characters; + static String get_invalid_node_name_characters(); String validate_node_name() const; String validate_identifier() const; String validate_filename() const; diff --git a/editor/scene_tree_editor.cpp b/editor/scene_tree_editor.cpp index 8ba6a1e610d4..808c058ecd2b 100644 --- a/editor/scene_tree_editor.cpp +++ b/editor/scene_tree_editor.cpp @@ -983,7 +983,7 @@ void SceneTreeEditor::_renamed() { String new_name = raw_new_name.validate_node_name(); if (new_name != raw_new_name) { - error->set_text(TTR("Invalid node name, the following characters are not allowed:") + "\n" + String::invalid_node_name_characters); + error->set_text(TTR("Invalid node name, the following characters are not allowed:") + "\n" + String::get_invalid_node_name_characters()); error->popup_centered(); if (new_name.is_empty()) { diff --git a/scene/main/node.cpp b/scene/main/node.cpp index d8375dbc9f14..9b3cc544600b 100644 --- a/scene/main/node.cpp +++ b/scene/main/node.cpp @@ -994,8 +994,29 @@ void Node::_validate_child_name(Node *p_child, bool p_force_human_readable) { if (!unique) { ERR_FAIL_COND(!node_hrcr_count.ref()); - String name = "@" + String(p_child->get_name()) + "@" + itos(node_hrcr_count.get()); - p_child->data.name = name; + // Optimized version of the code below: + // String name = "@" + String(p_child->get_name()) + "@" + itos(node_hrcr_count.get()); + uint32_t c = node_hrcr_count.get(); + String cn = p_child->get_class_name().operator String(); + const char32_t *cn_ptr = cn.ptr(); + uint32_t cn_length = cn.length(); + uint32_t c_chars = String::num_characters(c); + uint32_t len = 2 + cn_length + c_chars; + char32_t *str = (char32_t *)alloca(sizeof(char32_t) * (len + 1)); + uint32_t idx = 0; + str[idx++] = '@'; + for (uint32_t i = 0; i < cn_length; i++) { + str[idx++] = cn_ptr[i]; + } + str[idx++] = '@'; + idx += c_chars; + ERR_FAIL_COND(idx != len); + str[idx] = 0; + while (c) { + str[--idx] = '0' + (c % 10); + c /= 10; + } + p_child->data.name = String(str); } } } diff --git a/tests/core/string/test_string.h b/tests/core/string/test_string.h index 5d19b5a1649a..abe9f78ccc94 100644 --- a/tests/core/string/test_string.h +++ b/tests/core/string/test_string.h @@ -1678,8 +1678,8 @@ TEST_CASE("[String] validate_node_name") { String name_with_kana = U"Name with kana ゴドツ"; CHECK(name_with_kana.validate_node_name() == U"Name with kana ゴドツ"); - String name_with_invalid_chars = "Name with invalid characters :.@removed!"; - CHECK(name_with_invalid_chars.validate_node_name() == "Name with invalid characters removed!"); + String name_with_invalid_chars = "Name with invalid characters :.@%removed!"; + CHECK(name_with_invalid_chars.validate_node_name() == "Name with invalid characters ____removed!"); } TEST_CASE("[String] validate_identifier") {