Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++: In std=gnu++2b size-optimized builds std::string methods are inlined #67

Open
3 tasks done
rojer opened this issue Nov 10, 2024 · 9 comments
Open
3 tasks done

Comments

@rojer
Copy link

rojer commented Nov 10, 2024

Checklist

  • Checked the issue tracker for similar issues to ensure this is not a duplicate
  • Read the documentation to confirm the issue is not addressed there and your configuration is set correctly
  • Tested with the latest version to ensure the issue hasn't been fixed

How often does this bug occurs?

always

Expected behavior

I expected frequently used functions such as std::string::append not to be inlined, at least not in -Os mode.

Actual behavior (suspected bug)

Instead, when switching from gnu++17 to gnu++2b standard std::string functions such as append, resize, instead appear to be inlined. This alone is responsible for significant binary size increase for our application.

Error logs or terminal output

No response

Steps to reproduce the behavior

test file: https://gist.github.com/rojer/81900a923fdfa7e6853afe72fa2ea87b

to change the standard: edit tools/cmake/build.cmake, change the cxx_std variable.

it's obvious that in c++17 mode std::string methods are invoked while in c++2b mode they are inlined.

Project release version

14.2.0_20240906

System architecture

other (details in Additional context)

Operating system

Linux

Operating system version

Ubuntu 24.04

Shell

Bash

Additional context

IDF 5.2.2

@Lapshin
Copy link
Collaborator

Lapshin commented Nov 12, 2024

Hi @rojer, I appreciate your efforts in size optimization!

I took a look at your examples and that what I found - gnu++2b compared to gnu++17 has constexpr specifier for the most methods in stdlib. See macro _GLIBCXX20_CONSTEXPR

constexpr can explicitly inline functions and the issue appeared when the method called once in translation unit, otherwise gcc is smart enough to not inline the method and make a call... But yes, it's still an issue if you have lot of such cases. The first thing comes to my head is to redefine _GLIBCXX20_CONSTEXPR.

And your example could be built with xtensa-esp32-elf-c++ -Os test.cpp -std=gnu++2b -D_GLIBCXX20_CONSTEXPR="__attribute__((noinline))" -D__cpp_lib_constexpr_string="" -D__cpp_lib_string_resize_and_overwrite="". But I'm not sure that this approach is good to use...

@rojer
Copy link
Author

rojer commented Nov 12, 2024

ok, it's clear now and adheres to the C++20 standard but the tradeoff is definitely wrong for us - being able to construct std::strings in constexpr functions is not worth the bloat, at all.

i tried adding these flags to an IDF project build but the compiler got very angry at me :) probably some issue with quoting or something... can you help me apply this to an IDF project, just to see what effect it would have?

also, would you consider patching libstdc++ to add a proper way to override this during build?

something like

#ifndef GLIBCXX_STRING_CONSTEXPR
#define GLIBCXX_STRING_CONSTEXPR _GLIBCXX20_CONSTEXPR
#endif

and then use GLIBCXX_STRING_CONSTEXPR instead of _GLIBCXX20_CONSTEXPR directly. seems like a pretty straightforward change.

we'd then be able to add -DGLIBCXX_STRING_CONSTEXPR= to get back to C++17 behavior.

@Lapshin
Copy link
Collaborator

Lapshin commented Nov 13, 2024

@rojer , to apply flags to your projects please update your main/CMakeFiles.txt file with the following lines (insert after the line idf_component_register):

idf_build_set_property(CXX_COMPILE_OPTIONS "-D_GLIBCXX20_CONSTEXPR=__attribute__((noinline))" APPEND)
idf_build_set_property(CXX_COMPILE_OPTIONS "-D__cpp_lib_constexpr_string=" APPEND)
idf_build_set_property(CXX_COMPILE_OPTIONS "-D__cpp_lib_string_resize_and_overwrite=" APPEND)

I'm going to prepare a patch and open a ticket on GCC bugzilla to discuss the changes

@Lapshin
Copy link
Collaborator

Lapshin commented Nov 13, 2024

@rojer , I'm also interested to know if you've observed binary size increases only for string methods. It seems to be a problem for most std functions

@rojer
Copy link
Author

rojer commented Nov 13, 2024

you are most likely right, std::string was just the first thing i found and reported.

ok, i tried with the settings you suggested but surprisingly the size of binary went up, and by quite a lot (gcc14_cxx2b_opt):

$ ls -la build_Pro1.*/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1867712 Nov 13 14:25 build_Pro1.gcc13_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848880 Nov 13 14:23 build_Pro1.gcc14_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1864384 Nov 13 14:18 build_Pro1.gcc14_cxx2b/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 2004832 Nov 13 14:16 build_Pro1.gcc14_cxx2b_opt/objs/Pro1.bin

@Lapshin
Copy link
Collaborator

Lapshin commented Nov 13, 2024

@rojer , that's interesting. What result will be if you change __attribute__((noinline)) to __attribute__((cold)) ?

@rojer
Copy link
Author

rojer commented Nov 13, 2024

much better:

$ ll build_Pro1.*/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1867712 Nov 13 14:25 build_Pro1.gcc13_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848880 Nov 13 14:23 build_Pro1.gcc14_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1864384 Nov 13 14:18 build_Pro1.gcc14_cxx2b/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848720 Nov 13 19:00 build_Pro1.gcc14_cxx2b_opt_cold/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 2004832 Nov 13 14:16 build_Pro1.gcc14_cxx2b_opt/objs/Pro1.bin

even better than cxx17

@Lapshin
Copy link
Collaborator

Lapshin commented Nov 14, 2024

@rojer , thank you for sharing the results of the real application.

Could you please check one more option, change the strings:

-idf_build_set_property(CXX_COMPILE_OPTIONS "-D_GLIBCXX20_CONSTEXPR=__attribute__((cold))" APPEND)
-idf_build_set_property(CXX_COMPILE_OPTIONS "-D__cpp_lib_constexpr_string=" APPEND)
-idf_build_set_property(CXX_COMPILE_OPTIONS "-D__cpp_lib_string_resize_and_overwrite=" APPEND)
+idf_build_set_property(CXX_COMPILE_OPTIONS "-D_GLIBCXX20_CONSTEXPR=constexpr __attribute__((cold))" APPEND)

@rojer
Copy link
Author

rojer commented Nov 14, 2024

first, apologies: previous test was done incorrect, std was actually set to 17, so i renamed it to gcc14_cxx17_opt_cold. i re-did the test and cold result is a bit better than default but worse than 17. constexpr+cold (gcc14_cxx2b_opt_constexpr_cold) is worse.

$ ll build_Pro1*/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1867712 Nov 13 14:25 build_Pro1.gcc13_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848880 Nov 13 14:23 build_Pro1.gcc14_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848720 Nov 13 19:00 build_Pro1.gcc14_cxx17_opt_cold/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1864384 Nov 13 14:18 build_Pro1.gcc14_cxx2b/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1858336 Nov 14 12:47 build_Pro1.gcc14_cxx2b_opt_cold/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1863200 Nov 14 12:52 build_Pro1.gcc14_cxx2b_opt_constexpr_cold/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 2004832 Nov 13 14:16 build_Pro1.gcc14_cxx2b_opt/objs/Pro1.bin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants