From b221eab4260c471c37ff2aae2546fcfa6dd7ac58 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Fri, 20 May 2022 13:28:44 +0100 Subject: Variant memory pools Memory pools via PagedAllocator for Transform2D, Transform3D, Basis and AABB. --- core/config/engine.cpp | 4 +- core/core_globals.cpp | 35 ++++++++++++++++ core/core_globals.h | 44 ++++++++++++++++++++ core/io/logger.cpp | 3 +- core/string/print_string.cpp | 9 ++--- core/string/print_string.h | 2 - core/templates/paged_allocator.h | 10 ++++- core/variant/variant.cpp | 54 ++++++++++++++++++------- core/variant/variant.h | 19 +++++++++ core/variant/variant_internal.h | 14 +++++-- main/main.cpp | 5 ++- modules/gdscript/tests/gdscript_test_runner.cpp | 5 ++- platform/linuxbsd/detect_prime_x11.cpp | 5 +++ tests/test_macros.h | 13 +++--- tests/test_validate_testing.h | 5 ++- 15 files changed, 185 insertions(+), 42 deletions(-) create mode 100644 core/core_globals.cpp create mode 100644 core/core_globals.h diff --git a/core/config/engine.cpp b/core/config/engine.cpp index 782d369ae6..589691dce1 100644 --- a/core/config/engine.cpp +++ b/core/config/engine.cpp @@ -194,11 +194,11 @@ bool Engine::is_validation_layers_enabled() const { } void Engine::set_print_error_messages(bool p_enabled) { - _print_error_enabled = p_enabled; + CoreGlobals::print_error_enabled = p_enabled; } bool Engine::is_printing_error_messages() const { - return _print_error_enabled; + return CoreGlobals::print_error_enabled; } void Engine::add_singleton(const Singleton &p_singleton) { diff --git a/core/core_globals.cpp b/core/core_globals.cpp new file mode 100644 index 0000000000..45297b459f --- /dev/null +++ b/core/core_globals.cpp @@ -0,0 +1,35 @@ +/*************************************************************************/ +/* core_globals.cpp */ +/*************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/*************************************************************************/ +/* Copyright (c) 2007-2022 Juan Linietsky, Ariel Manzur. */ +/* Copyright (c) 2014-2022 Godot Engine contributors (cf. AUTHORS.md). */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/*************************************************************************/ + +#include "core_globals.h" + +bool CoreGlobals::leak_reporting_enabled = true; +bool CoreGlobals::print_line_enabled = true; +bool CoreGlobals::print_error_enabled = true; diff --git a/core/core_globals.h b/core/core_globals.h new file mode 100644 index 0000000000..c5e614dc0a --- /dev/null +++ b/core/core_globals.h @@ -0,0 +1,44 @@ +/*************************************************************************/ +/* core_globals.h */ +/*************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/*************************************************************************/ +/* Copyright (c) 2007-2022 Juan Linietsky, Ariel Manzur. */ +/* Copyright (c) 2014-2022 Godot Engine contributors (cf. AUTHORS.md). */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/*************************************************************************/ + +#ifndef CORE_GLOBALS_H +#define CORE_GLOBALS_H + +// Home for state needed from global functions +// that cannot be stored in Engine or OS due to e.g. circular includes + +class CoreGlobals { +public: + static bool leak_reporting_enabled; + static bool print_line_enabled; + static bool print_error_enabled; +}; + +#endif // CORE_GLOBALS_H diff --git a/core/io/logger.cpp b/core/io/logger.cpp index 5820ec0c09..b0f74f8db5 100644 --- a/core/io/logger.cpp +++ b/core/io/logger.cpp @@ -31,6 +31,7 @@ #include "logger.h" #include "core/config/project_settings.h" +#include "core/core_globals.h" #include "core/io/dir_access.h" #include "core/os/os.h" #include "core/os/time.h" @@ -41,7 +42,7 @@ #endif bool Logger::should_log(bool p_err) { - return (!p_err || _print_error_enabled) && (p_err || _print_line_enabled); + return (!p_err || CoreGlobals::print_error_enabled) && (p_err || CoreGlobals::print_line_enabled); } bool Logger::_flush_stdout_on_print = true; diff --git a/core/string/print_string.cpp b/core/string/print_string.cpp index f58486e0a5..592da58fe7 100644 --- a/core/string/print_string.cpp +++ b/core/string/print_string.cpp @@ -30,13 +30,12 @@ #include "print_string.h" +#include "core/core_globals.h" #include "core/os/os.h" #include static PrintHandlerList *print_handler_list = nullptr; -bool _print_line_enabled = true; -bool _print_error_enabled = true; void add_print_handler(PrintHandlerList *p_handler) { _global_lock(); @@ -70,7 +69,7 @@ void remove_print_handler(const PrintHandlerList *p_handler) { } void __print_line(String p_string) { - if (!_print_line_enabled) { + if (!CoreGlobals::print_line_enabled) { return; } @@ -87,7 +86,7 @@ void __print_line(String p_string) { } void __print_line_rich(String p_string) { - if (!_print_line_enabled) { + if (!CoreGlobals::print_line_enabled) { return; } @@ -178,7 +177,7 @@ void __print_line_rich(String p_string) { } void print_error(String p_string) { - if (!_print_error_enabled) { + if (!CoreGlobals::print_error_enabled) { return; } diff --git a/core/string/print_string.h b/core/string/print_string.h index 823e2c29e8..ca930a3a0f 100644 --- a/core/string/print_string.h +++ b/core/string/print_string.h @@ -56,8 +56,6 @@ String stringify_variants(Variant p_var, Args... p_args) { void add_print_handler(PrintHandlerList *p_handler); void remove_print_handler(const PrintHandlerList *p_handler); -extern bool _print_line_enabled; -extern bool _print_error_enabled; extern void __print_line(String p_string); extern void __print_line_rich(String p_string); extern void print_error(String p_string); diff --git a/core/templates/paged_allocator.h b/core/templates/paged_allocator.h index cf5911a847..43aab052fd 100644 --- a/core/templates/paged_allocator.h +++ b/core/templates/paged_allocator.h @@ -31,11 +31,14 @@ #ifndef PAGED_ALLOCATOR_H #define PAGED_ALLOCATOR_H +#include "core/core_globals.h" #include "core/os/memory.h" #include "core/os/spin_lock.h" +#include "core/string/ustring.h" #include "core/typedefs.h" #include +#include template class PagedAllocator { @@ -132,7 +135,12 @@ public: } ~PagedAllocator() { - ERR_FAIL_COND_MSG(allocs_available < pages_allocated * page_size, "Pages in use exist at exit in PagedAllocator"); + if (allocs_available < pages_allocated * page_size) { + if (CoreGlobals::leak_reporting_enabled) { + ERR_FAIL_COND_MSG(allocs_available < pages_allocated * page_size, String("Pages in use exist at exit in PagedAllocator: ") + String(typeid(T).name())); + } + return; + } reset(); } }; diff --git a/core/variant/variant.cpp b/core/variant/variant.cpp index ae92d7b5c4..1d81850182 100644 --- a/core/variant/variant.cpp +++ b/core/variant/variant.cpp @@ -39,6 +39,9 @@ #include "core/string/print_string.h" #include "core/variant/variant_parser.h" +PagedAllocator Variant::Pools::_bucket_small; +PagedAllocator Variant::Pools::_bucket_large; + String Variant::get_type_name(Variant::Type p_type) { switch (p_type) { case NIL: { @@ -1076,7 +1079,8 @@ void Variant::reference(const Variant &p_variant) { memnew_placement(_data._mem, Rect2i(*reinterpret_cast(p_variant._data._mem))); } break; case TRANSFORM2D: { - _data._transform2d = memnew(Transform2D(*p_variant._data._transform2d)); + _data._transform2d = (Transform2D *)Pools::_bucket_small.alloc(); + memnew_placement(_data._transform2d, Transform2D(*p_variant._data._transform2d)); } break; case VECTOR3: { memnew_placement(_data._mem, Vector3(*reinterpret_cast(p_variant._data._mem))); @@ -1087,20 +1091,20 @@ void Variant::reference(const Variant &p_variant) { case PLANE: { memnew_placement(_data._mem, Plane(*reinterpret_cast(p_variant._data._mem))); } break; - case AABB: { - _data._aabb = memnew(::AABB(*p_variant._data._aabb)); + _data._aabb = (::AABB *)Pools::_bucket_small.alloc(); + memnew_placement(_data._aabb, ::AABB(*p_variant._data._aabb)); } break; case QUATERNION: { memnew_placement(_data._mem, Quaternion(*reinterpret_cast(p_variant._data._mem))); - } break; case BASIS: { - _data._basis = memnew(Basis(*p_variant._data._basis)); - + _data._basis = (Basis *)Pools::_bucket_large.alloc(); + memnew_placement(_data._basis, Basis(*p_variant._data._basis)); } break; case TRANSFORM3D: { - _data._transform3d = memnew(Transform3D(*p_variant._data._transform3d)); + _data._transform3d = (Transform3D *)Pools::_bucket_large.alloc(); + memnew_placement(_data._transform3d, Transform3D(*p_variant._data._transform3d)); } break; // misc types @@ -1280,16 +1284,32 @@ void Variant::_clear_internal() { RECT2 */ case TRANSFORM2D: { - memdelete(_data._transform2d); + if (_data._transform2d) { + _data._transform2d->~Transform2D(); + Pools::_bucket_small.free((Pools::BucketSmall *)_data._transform2d); + _data._transform2d = nullptr; + } } break; case AABB: { - memdelete(_data._aabb); + if (_data._aabb) { + _data._aabb->~AABB(); + Pools::_bucket_small.free((Pools::BucketSmall *)_data._aabb); + _data._aabb = nullptr; + } } break; case BASIS: { - memdelete(_data._basis); + if (_data._basis) { + _data._basis->~Basis(); + Pools::_bucket_large.free((Pools::BucketLarge *)_data._basis); + _data._basis = nullptr; + } } break; case TRANSFORM3D: { - memdelete(_data._transform3d); + if (_data._transform3d) { + _data._transform3d->~Transform3D(); + Pools::_bucket_large.free((Pools::BucketLarge *)_data._transform3d); + _data._transform3d = nullptr; + } } break; // misc types @@ -2411,12 +2431,14 @@ Variant::Variant(const Plane &p_plane) { Variant::Variant(const ::AABB &p_aabb) { type = AABB; - _data._aabb = memnew(::AABB(p_aabb)); + _data._aabb = (::AABB *)Pools::_bucket_small.alloc(); + memnew_placement(_data._aabb, ::AABB(p_aabb)); } Variant::Variant(const Basis &p_matrix) { type = BASIS; - _data._basis = memnew(Basis(p_matrix)); + _data._basis = (Basis *)Pools::_bucket_large.alloc(); + memnew_placement(_data._basis, Basis(p_matrix)); } Variant::Variant(const Quaternion &p_quaternion) { @@ -2426,12 +2448,14 @@ Variant::Variant(const Quaternion &p_quaternion) { Variant::Variant(const Transform3D &p_transform) { type = TRANSFORM3D; - _data._transform3d = memnew(Transform3D(p_transform)); + _data._transform3d = (Transform3D *)Pools::_bucket_large.alloc(); + memnew_placement(_data._transform3d, Transform3D(p_transform)); } Variant::Variant(const Transform2D &p_transform) { type = TRANSFORM2D; - _data._transform2d = memnew(Transform2D(p_transform)); + _data._transform2d = (Transform2D *)Pools::_bucket_small.alloc(); + memnew_placement(_data._transform2d, Transform2D(p_transform)); } Variant::Variant(const Color &p_color) { diff --git a/core/variant/variant.h b/core/variant/variant.h index 872b374b13..1749702c61 100644 --- a/core/variant/variant.h +++ b/core/variant/variant.h @@ -51,6 +51,7 @@ #include "core/os/keyboard.h" #include "core/string/node_path.h" #include "core/string/ustring.h" +#include "core/templates/paged_allocator.h" #include "core/templates/rid.h" #include "core/variant/array.h" #include "core/variant/callable.h" @@ -128,6 +129,24 @@ public: }; private: + struct Pools { + union BucketSmall { + BucketSmall() {} + ~BucketSmall() {} + Transform2D _transform2d; + ::AABB _aabb; + }; + union BucketLarge { + BucketLarge() {} + ~BucketLarge() {} + Basis _basis; + Transform3D _transform3d; + }; + + static PagedAllocator _bucket_small; + static PagedAllocator _bucket_large; + }; + friend struct _VariantCall; friend class VariantInternal; // Variant takes 20 bytes when real_t is float, and 36 if double diff --git a/core/variant/variant_internal.h b/core/variant/variant_internal.h index e0cfb42e1e..583a7b56bd 100644 --- a/core/variant/variant_internal.h +++ b/core/variant/variant_internal.h @@ -36,6 +36,8 @@ // For use when you want to access the internal pointer of a Variant directly. // Use with caution. You need to be sure that the type is correct. class VariantInternal { + friend class Variant; + public: // Set type. _FORCE_INLINE_ static void initialize(Variant *v, Variant::Type p_type) { @@ -209,19 +211,23 @@ public: } _FORCE_INLINE_ static void init_transform2d(Variant *v) { - v->_data._transform2d = memnew(Transform2D); + v->_data._transform2d = (Transform2D *)Variant::Pools::_bucket_small.alloc(); + memnew_placement(v->_data._transform2d, Transform2D); v->type = Variant::TRANSFORM2D; } _FORCE_INLINE_ static void init_aabb(Variant *v) { - v->_data._aabb = memnew(AABB); + v->_data._aabb = (AABB *)Variant::Pools::_bucket_small.alloc(); + memnew_placement(v->_data._aabb, AABB); v->type = Variant::AABB; } _FORCE_INLINE_ static void init_basis(Variant *v) { - v->_data._basis = memnew(Basis); + v->_data._basis = (Basis *)Variant::Pools::_bucket_large.alloc(); + memnew_placement(v->_data._basis, Basis); v->type = Variant::BASIS; } _FORCE_INLINE_ static void init_transform(Variant *v) { - v->_data._transform3d = memnew(Transform3D); + v->_data._transform3d = (Transform3D *)Variant::Pools::_bucket_large.alloc(); + memnew_placement(v->_data._transform3d, Transform3D); v->type = Variant::TRANSFORM3D; } _FORCE_INLINE_ static void init_string_name(Variant *v) { diff --git a/main/main.cpp b/main/main.cpp index f7c192001b..b750321d01 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -31,6 +31,7 @@ #include "main.h" #include "core/config/project_settings.h" +#include "core/core_globals.h" #include "core/core_string_names.h" #include "core/crypto/crypto.h" #include "core/debugger/engine_debugger.h" @@ -1337,11 +1338,11 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph quiet_stdout = true; } if (bool(ProjectSettings::get_singleton()->get("application/run/disable_stderr"))) { - _print_error_enabled = false; + CoreGlobals::print_error_enabled = false; }; if (quiet_stdout) { - _print_line_enabled = false; + CoreGlobals::print_line_enabled = false; } Logger::set_flush_stdout_on_print(ProjectSettings::get_singleton()->get("application/run/flush_stdout_on_print")); diff --git a/modules/gdscript/tests/gdscript_test_runner.cpp b/modules/gdscript/tests/gdscript_test_runner.cpp index ff4832bde0..e3b956369d 100644 --- a/modules/gdscript/tests/gdscript_test_runner.cpp +++ b/modules/gdscript/tests/gdscript_test_runner.cpp @@ -36,6 +36,7 @@ #include "../gdscript_parser.h" #include "core/config/project_settings.h" +#include "core/core_globals.h" #include "core/core_string_names.h" #include "core/io/dir_access.h" #include "core/io/file_access_pack.h" @@ -142,8 +143,8 @@ GDScriptTestRunner::GDScriptTestRunner(const String &p_source_dir, bool p_init_l #endif // Enable printing to show results - _print_line_enabled = true; - _print_error_enabled = true; + CoreGlobals::print_line_enabled = true; + CoreGlobals::print_error_enabled = true; } GDScriptTestRunner::~GDScriptTestRunner() { diff --git a/platform/linuxbsd/detect_prime_x11.cpp b/platform/linuxbsd/detect_prime_x11.cpp index 42b7f68a5e..fb833ab5e6 100644 --- a/platform/linuxbsd/detect_prime_x11.cpp +++ b/platform/linuxbsd/detect_prime_x11.cpp @@ -177,6 +177,11 @@ int detect_prime() { } else { // In child, exit() here will not quit the engine. + // Prevent false leak reports as we will not be properly + // cleaning up these processes, and fork() makes a copy + // of all globals. + CoreGlobals::leak_reporting_enabled = false; + char string[201]; close(fdset[0]); diff --git a/tests/test_macros.h b/tests/test_macros.h index 6029a9cfc7..24aff618f5 100644 --- a/tests/test_macros.h +++ b/tests/test_macros.h @@ -31,6 +31,7 @@ #ifndef TEST_MACROS_H #define TEST_MACROS_H +#include "core/core_globals.h" #include "core/input/input_map.h" #include "core/object/message_queue.h" #include "core/variant/variant.h" @@ -53,12 +54,12 @@ // Temporarily disable error prints to test failure paths. // This allows to avoid polluting the test summary with error messages. -// The `_print_error_enabled` boolean is defined in `core/print_string.cpp` and +// The `print_error_enabled` boolean is defined in `core/core_globals.cpp` and // works at global scope. It's used by various loggers in `should_log()` method, // which are used by error macros which call into `OS::print_error`, effectively // disabling any error messages to be printed from the engine side (not tests). -#define ERR_PRINT_OFF _print_error_enabled = false; -#define ERR_PRINT_ON _print_error_enabled = true; +#define ERR_PRINT_OFF CoreGlobals::print_error_enabled = false; +#define ERR_PRINT_ON CoreGlobals::print_error_enabled = true; // Stringify all `Variant` compatible types for doctest output by default. // https://github.com/onqtam/doctest/blob/master/doc/markdown/stringification.md @@ -199,8 +200,8 @@ int register_test_command(String p_command, TestFunc p_function); // We toggle _print_error_enabled to prevent display server not supported warnings. #define SEND_GUI_MOUSE_MOTION_EVENT(m_object, m_local_pos, m_mask, m_modifers) \ { \ - bool errors_enabled = _print_error_enabled; \ - _print_error_enabled = false; \ + bool errors_enabled = CoreGlobals::print_error_enabled; \ + CoreGlobals::print_error_enabled = false; \ Ref event; \ event.instantiate(); \ event->set_position(m_local_pos); \ @@ -209,7 +210,7 @@ int register_test_command(String p_command, TestFunc p_function); _UPDATE_EVENT_MODIFERS(event, m_modifers); \ m_object->get_viewport()->push_input(event); \ MessageQueue::get_singleton()->flush(); \ - _print_error_enabled = errors_enabled; \ + CoreGlobals::print_error_enabled = errors_enabled; \ } // Utility class / macros for testing signals diff --git a/tests/test_validate_testing.h b/tests/test_validate_testing.h index 413a7e351d..1471a952cd 100644 --- a/tests/test_validate_testing.h +++ b/tests/test_validate_testing.h @@ -31,6 +31,7 @@ #ifndef TEST_VALIDATE_TESTING_H #define TEST_VALIDATE_TESTING_H +#include "core/core_globals.h" #include "core/os/os.h" #include "tests/test_macros.h" @@ -49,10 +50,10 @@ TEST_SUITE("Validate tests") { } TEST_CASE("Muting Godot error messages") { ERR_PRINT_OFF; - CHECK_MESSAGE(!_print_error_enabled, "Error printing should be disabled."); + CHECK_MESSAGE(!CoreGlobals::print_error_enabled, "Error printing should be disabled."); ERR_PRINT("Still waiting for Godot!"); // This should never get printed! ERR_PRINT_ON; - CHECK_MESSAGE(_print_error_enabled, "Error printing should be re-enabled."); + CHECK_MESSAGE(CoreGlobals::print_error_enabled, "Error printing should be re-enabled."); } TEST_CASE("Stringify Variant types") { Variant var; -- cgit v1.2.3