From 269b115d9cff1ad1bf1cd8675cd26c8a24065c89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Verschelde?= Date: Mon, 14 Aug 2023 13:02:40 +0200 Subject: [PATCH] SCons: Disable C++ exception handling Upon investigating the extremely slow MSVC build times in #80513, I noticed that while Godot policy is to never use exceptions, we weren't enforcing it with compiler flags, and thus still included exception handling code and stack unwinding. This is wasteful on multiple aspects: - Binary size: Around 20% binary size reduction with exceptions disabled for both MSVC and GCC binaries. - Compile time: * More than 50% build time reduction with MSVC. * 10% to 25% build time reduction with GCC + LTO. - Performance: Possibly, needs to be benchmarked. Since users may want to re-enable exceptions in their own thirdparty code or the libraries they compile with Godot, this behavior can be toggled with the `disable_exceptions` SCons option, which defaults to true. (cherry picked from commit 3907e53ff68643785df0066be64fddce9f79919c) --- SConstruct | 14 +++++++++++--- modules/denoise/SCsub | 9 +++++++++ modules/openxr/SCsub | 7 ++++--- platform/android/detect.py | 4 ---- platform/ios/detect.py | 6 ------ platform/web/detect.py | 9 +++------ tests/SCsub | 6 ++---- tests/core/io/test_image.h | 6 ++---- tests/scene/test_viewport.h | 2 +- 9 files changed, 32 insertions(+), 31 deletions(-) diff --git a/SConstruct b/SConstruct index 5dc0ed0fa28..791ff463039 100644 --- a/SConstruct +++ b/SConstruct @@ -193,6 +193,7 @@ opts.Add(BoolVariable("vulkan", "Enable the vulkan rendering driver", True)) opts.Add(BoolVariable("opengl3", "Enable the OpenGL/GLES3 rendering driver", True)) opts.Add(BoolVariable("openxr", "Enable the OpenXR driver", True)) opts.Add(BoolVariable("use_volk", "Use the volk library to load the Vulkan loader dynamically", True)) +opts.Add(BoolVariable("disable_exceptions", "Force disabling exception handling code", False)) opts.Add("custom_modules", "A list of comma-separated directory paths containing custom modules to build.", "") opts.Add(BoolVariable("custom_modules_recursive", "Detect custom modules recursively for each specified path.", True)) @@ -700,6 +701,16 @@ if selected_platform in platform_list: ) Exit(255) + # Disable exception handling. Godot doesn't use exceptions anywhere, and this + # saves around 20% of binary size and very significant build time (GH-80513). + if env["disable_exceptions"]: + if env.msvc: + env.Append(CPPDEFINES=[("_HAS_EXCEPTIONS", 0)]) + else: + env.Append(CCFLAGS=["-fno-exceptions"]) + elif env.msvc: + env.Append(CCFLAGS=["/EHsc"]) + # Configure compiler warnings if env.msvc: # MSVC if env["warnings"] == "no": @@ -729,9 +740,6 @@ if selected_platform in platform_list: ] ) - # Set exception handling model to avoid warnings caused by Windows system headers. - env.Append(CCFLAGS=["/EHsc"]) - if env["werror"]: env.Append(CCFLAGS=["/WX"]) env.Append(LINKFLAGS=["/WX"]) diff --git a/modules/denoise/SCsub b/modules/denoise/SCsub index 779ce165d26..967a511e1e0 100644 --- a/modules/denoise/SCsub +++ b/modules/denoise/SCsub @@ -109,6 +109,15 @@ env_oidn.AppendUnique(CPPDEFINES=["NDEBUG"]) # No assert() even in debug builds env_thirdparty = env_oidn.Clone() env_thirdparty.disable_warnings() + +if env["disable_exceptions"]: + # OIDN hard-requires exceptions, so we re-enable them here. + if env.msvc and ("_HAS_EXCEPTIONS", 0) in env_thirdparty["CPPDEFINES"]: + env_thirdparty["CPPDEFINES"].remove(("_HAS_EXCEPTIONS", 0)) + env_thirdparty.AppendUnique(CCFLAGS=["/EHsc"]) + elif not env.msvc and "-fno-exceptions" in env_thirdparty["CCFLAGS"]: + env_thirdparty["CCFLAGS"].remove("-fno-exceptions") + env_thirdparty.add_source_files(thirdparty_obj, thirdparty_sources) env.modules_sources += thirdparty_obj diff --git a/modules/openxr/SCsub b/modules/openxr/SCsub index d5abbe4c725..53faa65e97b 100644 --- a/modules/openxr/SCsub +++ b/modules/openxr/SCsub @@ -47,10 +47,11 @@ if env["builtin_openxr"]: env_thirdparty = env_openxr.Clone() env_thirdparty.disable_warnings() - env_thirdparty.AppendUnique(CPPDEFINES=["DISABLE_STD_FILESYSTEM"]) - if "-fno-exceptions" in env_thirdparty["CXXFLAGS"]: - env_thirdparty["CXXFLAGS"].remove("-fno-exceptions") + env_thirdparty.AppendUnique(CPPDEFINES=["DISABLE_STD_FILESYSTEM"]) + if env["disable_exceptions"]: + env_thirdparty.AppendUnique(CPPDEFINES=["XRLOADER_DISABLE_EXCEPTION_HANDLING", ("JSON_USE_EXCEPTION", 0)]) + env_thirdparty.Append(CPPPATH=[thirdparty_dir + "/src/loader"]) # add in external jsoncpp dependency diff --git a/platform/android/detect.py b/platform/android/detect.py index 2860898e5c5..33c65657899 100644 --- a/platform/android/detect.py +++ b/platform/android/detect.py @@ -170,10 +170,6 @@ def configure(env: "Environment"): env["RANLIB"] = compiler_path + "/llvm-ranlib" env["AS"] = compiler_path + "/clang" - # Disable exceptions on template builds - if not env.editor_build: - env.Append(CXXFLAGS=["-fno-exceptions"]) - env.Append( CCFLAGS=( "-fpic -ffunction-sections -funwind-tables -fstack-protector-strong -fvisibility=hidden -fno-strict-aliasing".split() diff --git a/platform/ios/detect.py b/platform/ios/detect.py index a01f4b55db6..e11c0b7d917 100644 --- a/platform/ios/detect.py +++ b/platform/ios/detect.py @@ -30,7 +30,6 @@ def get_opts(): ), ("IOS_SDK_PATH", "Path to the iOS SDK", ""), BoolVariable("ios_simulator", "Build for iOS Simulator", False), - BoolVariable("ios_exceptions", "Enable exceptions", False), ("ios_triple", "Triple for ios toolchain", ""), ] @@ -138,11 +137,6 @@ def configure(env: "Environment"): env.Append(ASFLAGS=["-arch", "arm64"]) env.Append(CPPDEFINES=["NEED_LONG_INT"]) - if env["ios_exceptions"]: - env.Append(CCFLAGS=["-fexceptions"]) - else: - env.Append(CCFLAGS=["-fno-exceptions"]) - # Temp fix for ABS/MAX/MIN macros in iOS SDK blocking compilation env.Append(CCFLAGS=["-Wno-ambiguous-macro"]) diff --git a/platform/web/detect.py b/platform/web/detect.py index 997e30e9e10..781560cf610 100644 --- a/platform/web/detect.py +++ b/platform/web/detect.py @@ -97,12 +97,9 @@ def configure(env: "Environment"): if env["use_assertions"]: env.Append(LINKFLAGS=["-s", "ASSERTIONS=1"]) - if env.editor_build: - if env["initial_memory"] < 64: - print('Note: Forcing "initial_memory=64" as it is required for the web editor.') - env["initial_memory"] = 64 - else: - env.Append(CPPFLAGS=["-fno-exceptions"]) + if env.editor_build and env["initial_memory"] < 64: + print('Note: Forcing "initial_memory=64" as it is required for the web editor.') + env["initial_memory"] = 64 env.Append(LINKFLAGS=["-s", "INITIAL_MEMORY=%sMB" % env["initial_memory"]]) diff --git a/tests/SCsub b/tests/SCsub index c59ce69b925..d96a1142e4d 100644 --- a/tests/SCsub +++ b/tests/SCsub @@ -13,10 +13,8 @@ env_tests = env.Clone() if env_tests["platform"] == "windows": env_tests.Append(CPPDEFINES=[("DOCTEST_THREAD_LOCAL", "")]) -# Increase number of addressable sections in object files -# due to doctest's heavy use of templates and macros. -if env_tests.msvc: - env_tests.Append(CCFLAGS=["/bigobj"]) +if env["disable_exceptions"]: + env_tests.Append(CPPDEFINES=["DOCTEST_CONFIG_NO_EXCEPTIONS_BUT_WITH_ALL_ASSERTS"]) env_tests.add_source_files(env.tests_sources, "*.cpp") diff --git a/tests/core/io/test_image.h b/tests/core/io/test_image.h index d6eaa8bd775..4f43f184523 100644 --- a/tests/core/io/test_image.h +++ b/tests/core/io/test_image.h @@ -252,9 +252,7 @@ TEST_CASE("[Image] Modifying pixels of an image") { for (const Rect2i &rect : rects) { Ref img = memnew(Image(img_width, img_height, false, Image::FORMAT_RGBA8)); - CHECK_NOTHROW_MESSAGE( - img->fill_rect(rect, Color(1, 1, 1, 1)), - "fill_rect() shouldn't throw for any rect."); + img->fill_rect(rect, Color(1, 1, 1, 1)); for (int y = 0; y < img->get_height(); y++) { for (int x = 0; x < img->get_width(); x++) { if (rect.abs().has_point(Point2(x, y))) { @@ -307,7 +305,7 @@ TEST_CASE("[Image] Modifying pixels of an image") { // Pre-multiply Alpha then Convert from RGBA to L8, checking alpha { Ref gray_image = memnew(Image(3, 3, false, Image::FORMAT_RGBA8)); - CHECK_NOTHROW_MESSAGE(gray_image->fill_rect(Rect2i(0, 0, 3, 3), Color(1, 1, 1, 0)), "fill_rect() shouldn't throw for any rect."); + gray_image->fill_rect(Rect2i(0, 0, 3, 3), Color(1, 1, 1, 0)); gray_image->set_pixel(1, 1, Color(1, 1, 1, 1)); gray_image->set_pixel(1, 2, Color(0.5, 0.5, 0.5, 0.5)); gray_image->set_pixel(2, 1, Color(0.25, 0.05, 0.5, 1.0)); diff --git a/tests/scene/test_viewport.h b/tests/scene/test_viewport.h index d76fc401252..fb06f144a46 100644 --- a/tests/scene/test_viewport.h +++ b/tests/scene/test_viewport.h @@ -222,7 +222,7 @@ TEST_CASE("[SceneTree][Viewport] Controls and InputEvent handling") { SUBCASE("[Viewport][GuiInputEvent] nullptr as argument doesn't lead to a crash.") { ERR_PRINT_OFF; - CHECK_NOTHROW(root->push_input(nullptr)); + root->push_input(nullptr); ERR_PRINT_ON; }