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

Reduce library size #2299

Closed
vitaut opened this issue May 20, 2021 · 15 comments
Closed

Reduce library size #2299

vitaut opened this issue May 20, 2021 · 15 comments

Comments

@vitaut
Copy link
Contributor

vitaut commented May 20, 2021

Need to revisit library size optimizations (https://www.zverovich.net/2020/05/21/reducing-library-size.html) since there was a bit of a regression compared to 7.0:

% clang++ -std=c++17  -I include -O2 -DNDEBUG test.cc src/format.cc
% strip a.out
% ls -l a.out
-rwxr-xr-x  1 viz  staff  193960 May 20 11:50 a.out

For comparison 7.0 generates

-rwxr-xr-x  1 viz  staff  127472 May 20 11:53 a.out

with the latest clang and libc++. Looks like some of the regression is due to newer clang and standard library.

@vitaut
Copy link
Contributor Author

vitaut commented May 20, 2021

7.1 is ~20k bigger because of adding Dragonbox (0c8ffe9). Maybe we can reuse powers of 10 table between Grisu and Dragonbox.

@vitaut
Copy link
Contributor Author

vitaut commented May 20, 2021

Most of the regression happened between 7.1.0 and 7.1.1 so should be easy to track down.

@vitaut
Copy link
Contributor Author

vitaut commented May 20, 2021

The regression between 7.1.0 and 7.1.1 was to preserve ABI compatibility (4081b2f) and is fixed now.

@vitaut
Copy link
Contributor Author

vitaut commented May 20, 2021

~30k regression was caused by b268f88.

@sergiud
Copy link
Contributor

sergiud commented May 21, 2021

Following #2293, I would suggest enabling -fvisibility=hidden and -fvisibility-inlines-hidden by default in CMakeLists.txt:

set (CMAKE_CXX_VISIBILITY_PRESET hidden)
set (CMAKE_VISIBILITY_INLINES_HIDDEN ON)
$ du -h build-before/libfmt.so.7.1.4
244K
$ du -h build-after/libfmt.so.7.1.4
212K

I can provide a PR.

@vitaut
Copy link
Contributor Author

vitaut commented May 21, 2021

I would suggest enabling -fvisibility=hidden and -fvisibility-inlines-hidden by default

Good idea but I would suggest using target properties instead of global CMake flags.

I can provide a PR.

Please do.

@vitaut
Copy link
Contributor Author

vitaut commented May 21, 2021

With b268f88 regression fixed:

% clang++ -std=c++17  -I include -O2 -DNDEBUG test.cc src/format.cc && strip a.out && ls -l a.out
-rwxr-xr-x  1 viz  staff  161168 May 21 06:27 a.out

@sergiud
Copy link
Contributor

sergiud commented May 21, 2021

CMake target properties would not provide any benefit here. On contrary, users will not be able to override the default choice by passing CMAKE_CXX_VISIBILITY_PRESET and CMAKE_VISIBILITY_INLINES_HIDDEN as cache variables.

@vitaut
Copy link
Contributor Author

vitaut commented May 21, 2021

OK, just make sure that it works well with fmt used as a subproject, i.e. something like

if (FMT_MASTER_PROJECT AND NOT CMAKE_BUILD_TYPE)

@vitaut
Copy link
Contributor Author

vitaut commented May 21, 2021

The remaining two ~15k regressions (each):

@alexezeder
Copy link
Contributor

Brag time 😉

Some of the regressions can be found here - https://alexezeder.github.io/fmt_bnchmrk/
There are the results generated by fmt_bnchmrk_gnrtr with fmt_bnchmrk for {fmt}.
But I don't want to make any promises about the website (aka Pages) because it's running on Raspberry Pi, and if it dies, then there won't be any updates for the website. But the code will be available, I hope, forever.

@vitaut
Copy link
Contributor Author

vitaut commented May 30, 2021

This is amazing, thanks for putting it together.

@vitaut
Copy link
Contributor Author

vitaut commented May 30, 2021

After 5a2b88f:

% clang++ -std=c++17 -I include -O2 -DNDEBUG test.cc src/format.cc && strip a.out && ls -l a.out
-rwxr-xr-x  1 viz  staff  143632 May 30 06:17 a.out

@vitaut
Copy link
Contributor Author

vitaut commented May 30, 2021

This is slightly smaller than 7.1.0 so I think it's good enough and we could look at floating point separately.

@vitaut vitaut closed this as completed May 30, 2021
@vitaut
Copy link
Contributor Author

vitaut commented May 30, 2021

Here are results with LTO and -Os:

% clang++ -std=c++17 -I include -Os -flto -DNDEBUG test.cc src/format.cc && strip a.out && ls -l a.out
-rwxr-xr-x  1 viz  staff  101776 May 30 06:23 a.out

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

3 participants