From f0c48c674817e623c124efc12e8bce4961d8a3f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 31 Jul 2024 11:27:43 +0200 Subject: [PATCH 1/3] [wip] Utility: deduplicate __FILE__ in internal assertion macros. Previously, it was joined together with the actual stringified condition, leading to the file path being repeated several times in the resulting binary. By making it a separate literal it has a null terminator, which makes the compiler able to deduplicate all such occurences to just one. In a stripped libCorradeUtility.so this results in the binary getting about 4 kB smaller (641 kB before, 637 after). Looking with `strings`, `grep` and `wc -c`, the internal assertion messages were about 8 kB before, now they're together with deduplicated filenames ~4 kB. Detailed Bloaty report however shows that the actual code size increased significantly as well, offseting the gains: FILE SIZE VM SIZE -------------- -------------- +0.7% +2.53Ki +0.7% +2.53Ki .text [ = ] 0 +33% +8 .data -0.1% -8 -0.1% -8 .eh_frame_hdr -0.2% -8 -0.2% -8 .got.plt -0.1% -12 -0.1% -12 .gcc_except_table -0.0% -16 -0.0% -16 .dynstr -0.2% -16 -0.2% -16 .plt -0.1% -24 -0.1% -24 .dynsym -0.2% -24 -0.2% -24 .rela.plt -0.1% -64 -0.1% -64 .eh_frame -22.9% -1.81Ki [ = ] 0 [Unmapped] -14.3% -4.56Ki -14.3% -4.56Ki .rodata -0.6% -4.01Ki -0.3% -2.19Ki TOTAL Testing with libMagnumTrade.so, which has 8 kB of internal assertions, and ~2 kB after, didn't show any meaningful change however: FILE SIZE VM SIZE -------------- -------------- +0.7% +4.00Ki +0.7% +4.00Ki .text +34% +1.72Ki [ = ] 0 [Unmapped] -6.4% -5.72Ki -6.4% -5.72Ki .rodata [ = ] 0 -0.2% -1.72Ki TOTAL And neither did with libMagnumWhee.so, the new UI library replacement, where internal assertions were 27 kB (!) before, and ~12 kB after. All the gains were eaten up by increases in code size, and what I suspect is two extra 64-bit pointers for the new strings in each assertions. FILE SIZE VM SIZE -------------- -------------- +2.7% +8.72Ki +2.7% +8.72Ki .text +223% +5.24Ki [ = ] 0 [Unmapped] +0.4% +264 +0.4% +264 .eh_frame +0.1% +80 +0.1% +80 .dynstr +0.4% +48 +0.4% +48 .eh_frame_hdr +0.1% +24 +0.1% +24 .dynsym +0.1% +8 +0.1% +8 .gnu.hash +0.3% +8 +0.3% +8 .gnu.version -1.1% -197 -1.1% -197 .gcc_except_table -5.6% -14.2Ki -5.6% -14.2Ki .rodata [ = ] 0 -0.6% -5.24Ki TOTAL The conclusion is that this change is probably not worth doing in its current form. Putting aside until I get a better idea. - Maybe a radical simplification of the Debug helper (with a fast path for printing just char*) would help? - Maybe splitting to just two strings instead of 3 (assertion message, file, line) could help, and letting the internals format it somehow? - Maybe just sidestep Debug altogether in these and just std::puts() or whatever? Most of the formatting functionality isn't needed anyway, all we have is char pointers, although it should still be capable of redirecting default output to somewhere else (such as adb logcat on Android). - I had an idea to insert `\0`s in the literal, remember the literal size and then split them up internally, but that would only allow deduplication with __FILE__ being used standalone somewhere else, not any other asserts. --- src/Corrade/Utility/Assert.h | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/Corrade/Utility/Assert.h b/src/Corrade/Utility/Assert.h index a020acfcc..dcff35201 100644 --- a/src/Corrade/Utility/Assert.h +++ b/src/Corrade/Utility/Assert.h @@ -408,10 +408,17 @@ You can override this implementation by placing your own #elif defined(CORRADE_STANDARD_ASSERT) #define CORRADE_INTERNAL_ASSERT(condition) assert(condition) #else +/* The __FILE__ is deliberately printed separately instead of joined with the + rest of the string literal to deduplicate it in the binary. As the full file + path is usually used by the buildsystem, it can get rather long, and the + savings from deduplicating significantly outweigh the extra code size, + especially in assertion-heavy code. OTOH the "Assertion" and "failed at" is + joined with the stringified condition, as they're relatively short and the + extra code size would likely be more than the savings. */ #define CORRADE_INTERNAL_ASSERT(condition) \ do { \ if(!(condition)) { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #condition " failed at " __FILE__ ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #condition " failed at" << __FILE__ << Corrade::Utility::Debug::nospace << ":" CORRADE_LINE_STRING; \ std::abort(); \ } \ } while(false) @@ -451,9 +458,10 @@ You can override this implementation by placing your own assert(!#condition); \ }(), 0)) #else +/* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ #define CORRADE_INTERNAL_CONSTEXPR_ASSERT(condition) \ static_cast((condition) ? 0 : ([&]() { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #condition " failed at " __FILE__ ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #condition " failed at" << __FILE__ << Corrade::Utility::Debug::nospace << ":" CORRADE_LINE_STRING; \ std::abort(); \ }(), 0)) #endif @@ -485,10 +493,11 @@ You can override this implementation by placing your own #elif defined(CORRADE_STANDARD_ASSERT) #define CORRADE_INTERNAL_ASSERT_OUTPUT(call) assert(call) #else +/* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ #define CORRADE_INTERNAL_ASSERT_OUTPUT(call) \ do { \ if(!(call)) { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #call " failed at " __FILE__ ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #call " failed at" << __FILE__ << Corrade::Utility::Debug::nospace << ":" CORRADE_LINE_STRING; \ std::abort(); \ } \ } while(false) @@ -559,6 +568,8 @@ You can override this implementation by placing your own #elif defined(CORRADE_STANDARD_ASSERT) #define CORRADE_INTERNAL_ASSERT_EXPRESSION(...) Corrade::Utility::Implementation::assertExpression(__VA_ARGS__) #else +/* Unlike other INTERNAL_ASSERT macros which print __FILE__ separately to + deduplicate it in the binary, here it's unfortunately not possible */ #define CORRADE_INTERNAL_ASSERT_EXPRESSION(...) Corrade::Utility::Implementation::assertExpression(__VA_ARGS__, "Assertion " #__VA_ARGS__ " failed at " __FILE__ ":" CORRADE_LINE_STRING) #endif #endif @@ -602,9 +613,10 @@ You can override this implementation by placing your own #elif defined(CORRADE_STANDARD_ASSERT) #define CORRADE_INTERNAL_ASSERT_UNREACHABLE() assert(!"unreachable code") #else +/* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ #define CORRADE_INTERNAL_ASSERT_UNREACHABLE() \ do { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Reached unreachable code at " __FILE__ ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Reached unreachable code at" << __FILE__ << Corrade::Utility::Debug::nospace << ":" CORRADE_LINE_STRING; \ std::abort(); \ } while(false) #endif From da3b473f2604c8d081962a0c82f75ff79e160e39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 31 Jul 2024 12:19:16 +0200 Subject: [PATCH 2/3] [wip] Utility: try to pass NoSpace to the constructor instead. This is one less function call, however it doesn't really have any effect either. The total change with libMagnumWhee.so is now this: FILE SIZE VM SIZE -------------- -------------- +3.1% +9.92Ki +3.1% +9.92Ki .text +155% +3.65Ki [ = ] 0 [Unmapped] +0.6% +368 +0.6% +368 .eh_frame +0.1% +80 +0.1% +80 .dynstr +0.5% +64 +0.5% +64 .eh_frame_hdr +0.1% +24 +0.1% +24 .dynsym +0.1% +8 +0.1% +8 .gnu.hash +0.3% +8 +0.3% +8 .gnu.version -0.3% -46 -0.3% -46 .gcc_except_table -5.5% -14.1Ki -5.5% -14.1Ki .rodata [ = ] 0 -0.4% -3.65Ki TOTAL --- src/Corrade/Utility/Assert.h | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Corrade/Utility/Assert.h b/src/Corrade/Utility/Assert.h index dcff35201..7d0c61d41 100644 --- a/src/Corrade/Utility/Assert.h +++ b/src/Corrade/Utility/Assert.h @@ -418,7 +418,7 @@ You can override this implementation by placing your own #define CORRADE_INTERNAL_ASSERT(condition) \ do { \ if(!(condition)) { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #condition " failed at" << __FILE__ << Corrade::Utility::Debug::nospace << ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #condition " failed at " << __FILE__ << ":" CORRADE_LINE_STRING; \ std::abort(); \ } \ } while(false) @@ -461,7 +461,7 @@ You can override this implementation by placing your own /* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ #define CORRADE_INTERNAL_CONSTEXPR_ASSERT(condition) \ static_cast((condition) ? 0 : ([&]() { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #condition " failed at" << __FILE__ << Corrade::Utility::Debug::nospace << ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #condition " failed at " << __FILE__ << ":" CORRADE_LINE_STRING; \ std::abort(); \ }(), 0)) #endif @@ -497,7 +497,7 @@ You can override this implementation by placing your own #define CORRADE_INTERNAL_ASSERT_OUTPUT(call) \ do { \ if(!(call)) { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Assertion " #call " failed at" << __FILE__ << Corrade::Utility::Debug::nospace << ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #call " failed at " << __FILE__ << ":" CORRADE_LINE_STRING; \ std::abort(); \ } \ } while(false) @@ -512,9 +512,9 @@ namespace Corrade { namespace Utility { namespace Implementation { return Corrade::Utility::forward(value); } #else - template T assertExpression(T&& value, const char* message) { + template T assertExpression(T&& value, const char* message, const char* file, const char* line) { if(!value) { - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << message; + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << message << file << line; std::abort(); } @@ -568,9 +568,8 @@ You can override this implementation by placing your own #elif defined(CORRADE_STANDARD_ASSERT) #define CORRADE_INTERNAL_ASSERT_EXPRESSION(...) Corrade::Utility::Implementation::assertExpression(__VA_ARGS__) #else -/* Unlike other INTERNAL_ASSERT macros which print __FILE__ separately to - deduplicate it in the binary, here it's unfortunately not possible */ -#define CORRADE_INTERNAL_ASSERT_EXPRESSION(...) Corrade::Utility::Implementation::assertExpression(__VA_ARGS__, "Assertion " #__VA_ARGS__ " failed at " __FILE__ ":" CORRADE_LINE_STRING) +/* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ +#define CORRADE_INTERNAL_ASSERT_EXPRESSION(...) Corrade::Utility::Implementation::assertExpression(__VA_ARGS__, "Assertion " #__VA_ARGS__ " failed at ", __FILE__, ":" CORRADE_LINE_STRING) #endif #endif @@ -616,7 +615,7 @@ You can override this implementation by placing your own /* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ #define CORRADE_INTERNAL_ASSERT_UNREACHABLE() \ do { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput()} << "Reached unreachable code at" << __FILE__ << Corrade::Utility::Debug::nospace << ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Reached unreachable code at " << __FILE__ << ":" CORRADE_LINE_STRING; \ std::abort(); \ } while(false) #endif From ba2e85ce8df8c3624fcdf7ea52ceb02020735b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 31 Jul 2024 12:32:37 +0200 Subject: [PATCH 3/3] [wip] Utility: try to pass line as an integer, not a string. This further reduces the string table, but doesn't help in making libMagnumWhee.so any smaller either. Plus a consequence is that using an assertion pulls in all the number-to-string conversion machinery, which is absolutely undesitable. FILE SIZE VM SIZE -------------- -------------- +2.9% +9.38Ki +2.9% +9.38Ki .text +218% +5.13Ki [ = ] 0 [Unmapped] +0.6% +368 +0.6% +368 .eh_frame +0.1% +80 +0.1% +80 .dynstr +0.5% +64 +0.5% +64 .eh_frame_hdr +0.1% +24 +0.1% +24 .dynsym +0.1% +8 +0.1% +8 .gnu.hash +0.3% +8 +0.3% +8 .gnu.version -0.3% -46 -0.3% -46 .gcc_except_table -5.9% -15.0Ki -5.9% -15.0Ki .rodata [ = ] 0 -0.6% -5.13Ki TOTAL --- src/Corrade/Utility/Assert.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Corrade/Utility/Assert.h b/src/Corrade/Utility/Assert.h index 7d0c61d41..94fceb510 100644 --- a/src/Corrade/Utility/Assert.h +++ b/src/Corrade/Utility/Assert.h @@ -36,7 +36,6 @@ #include #include "Corrade/Utility/Debug.h" -#include "Corrade/Utility/Macros.h" /* CORRADE_LINE_STRING */ #elif !defined(NDEBUG) #include #endif @@ -418,7 +417,7 @@ You can override this implementation by placing your own #define CORRADE_INTERNAL_ASSERT(condition) \ do { \ if(!(condition)) { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #condition " failed at " << __FILE__ << ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #condition " failed at " << __FILE__ ":" << __LINE__; \ std::abort(); \ } \ } while(false) @@ -461,7 +460,7 @@ You can override this implementation by placing your own /* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ #define CORRADE_INTERNAL_CONSTEXPR_ASSERT(condition) \ static_cast((condition) ? 0 : ([&]() { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #condition " failed at " << __FILE__ << ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #condition " failed at " << __FILE__ ":" << __LINE__; \ std::abort(); \ }(), 0)) #endif @@ -497,7 +496,7 @@ You can override this implementation by placing your own #define CORRADE_INTERNAL_ASSERT_OUTPUT(call) \ do { \ if(!(call)) { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #call " failed at " << __FILE__ << ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Assertion " #call " failed at " << __FILE__ ":" << __LINE__; \ std::abort(); \ } \ } while(false) @@ -512,7 +511,7 @@ namespace Corrade { namespace Utility { namespace Implementation { return Corrade::Utility::forward(value); } #else - template T assertExpression(T&& value, const char* message, const char* file, const char* line) { + template T assertExpression(T&& value, const char* message, const char* file, int line) { if(!value) { Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << message << file << line; std::abort(); @@ -569,7 +568,7 @@ You can override this implementation by placing your own #define CORRADE_INTERNAL_ASSERT_EXPRESSION(...) Corrade::Utility::Implementation::assertExpression(__VA_ARGS__) #else /* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ -#define CORRADE_INTERNAL_ASSERT_EXPRESSION(...) Corrade::Utility::Implementation::assertExpression(__VA_ARGS__, "Assertion " #__VA_ARGS__ " failed at ", __FILE__, ":" CORRADE_LINE_STRING) +#define CORRADE_INTERNAL_ASSERT_EXPRESSION(...) Corrade::Utility::Implementation::assertExpression(__VA_ARGS__, "Assertion " #__VA_ARGS__ " failed at ", __FILE__ ":", __LINE__) #endif #endif @@ -615,7 +614,7 @@ You can override this implementation by placing your own /* See CORRADE_INTERNAL_ASSERT() for why __FILE__ is printed separately */ #define CORRADE_INTERNAL_ASSERT_UNREACHABLE() \ do { \ - Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Reached unreachable code at " << __FILE__ << ":" CORRADE_LINE_STRING; \ + Corrade::Utility::Error{Corrade::Utility::Error::defaultOutput(), Corrade::Utility::Debug::Flag::NoSpace} << "Reached unreachable code at " << __FILE__ ":" << __LINE__; \ std::abort(); \ } while(false) #endif