First-time contributor -- speeding up `std::unique_ptr` compilation

[Vittorio Romeo]

Thank you, Jonathan, for the very thorough explanation and for pointing out that I completely forgot to consider ABI stability in this case.

I still think that it might be worthwhile to attempt some work in this area as multiple projects I work on have various TUs using unique_ptr but not tuple. I will look into the refactoring you suggested.


Vittorio Romeo
https://vittorioromeo.info
________________________________
From: Jonathan Wakely <jwakely.gcc@gmail.com>
Sent: Friday, June 24, 2022 9:22:56 AM
To: mail@vittorioromeo.com <mail@vittorioromeo.com>
Cc: libstdc++ <libstdc++@gcc.gnu.org>
Subject: Re: First-time contributor -- speeding up `std::unique_ptr` compilation



On Fri, 24 Jun 2022, 07:20 Jonathan Wakely, <jwakely.gcc@gmail.com<mailto:jwakely.gcc@gmail.com>> wrote:


On Fri, 24 Jun 2022, 01:19 , <mail@vittorioromeo.com<mailto:mail@vittorioromeo.com>> wrote:
Hello everyone,
Hope you are all doing well.

I would like to begin by saying that I am terribly unfamiliar with mailing lists and I've always found them cumbersome to use -- I tend to do most of my development via GitHub PRs and Discord. Therefore, please forgive me if this is not the right place to discuss "how to contribute", and also forgive me for not being able to create a proper patch file.

My goal is to generally improve the compilation speed of C++ programs using the standard library. In personal projects and open-source projects I maintain, I've achieved good results by benchmarking compilation via ClangBuildAnalyzer, figuring out the bottlenecks, and optimizing them. While working on the SFML project, I noticed that the `std::unique_ptr` template was taking a lot of time to be instantiated using the latest version of libstdc++.

I investigated a bit, and figured out that the `<tuple>` include and `std::tuple` instantiation were the culprit. I then replaced them with a handmade workaround in my system libraries, recompiled SFML, and noticed that `std::unique_ptr`'s instantiation time became negligible as a result of my changes. As a side benefit, we also avoid including the '<tuple>' header altogether, reducing the risk of unintended transitive includes. I created a PR on the GCC GitHub mirror as a way to observe the diff in a nice interface and to discuss my changes, and to see if the maintainers generally think that this is a good idea. You can see the diff here:
https://github.com/gcc-mirror/gcc/pull/67/files?diff=split&w=1

In terms of rough measurements, the average instantiation time of `std::unique_ptr` went from 22ms to 4ms. This was measured over the entire SFML codebase using clang++ 14.x with the `-ftime-trace` flag, plus the aforementioned ClangBuildAnalyzer tool.

A few questions:

  1.  Is this improvement something that the libstdc++ maintainers would want to merge in? Of course, the changes need to be cleaned up and adapted to GCC's coding style.

No, it would be a ABI break. It would change the layout for a deleter with the 'final' specifier.

It also looks like quite a maintenance headache "just" for improved compilation time. The need for two implementations (with and without the attribute) that both need to be kept ABI compatible with std::tuple doesn't seem like a good trade off and a good use of maintainer effort.

I think a more maintainable (and ABI compatible) approach would be to move the _Head_base templates to a new header and include that in bits/unique_ptr.h, without the rest of <tuple>. That will also allow the gdb pretty printers to continue working, whereas your suggestion would mean rewriting them.

I know you are concerned about build times, but correctness, ABI compatibility, and to a lesser extent maintainability all matter more. Build times for microbenchmarks can be worth checking, but bear in mind that your changes actually mean *more* code needs to be compiled for a program that already uses std::tuple anyway. You're not removing any code from the library or making the existing tuple code faster to compile, only adding more code.





  2.  If so, what is the easiest way to create a patch in the format that you are used to and test it against libstdc++'s test suite? I have never contributed to libstdc++ or GCC before.

You need to test it first, before you create a patch. Build your modified sources as per https://gcc.gnu.org/wiki/InstallingGCC (but starting with your git clone instead of a release tarball).

You can use --enable-languages=c++ to avoid building more of the compiler then you need for libstdc++ testing.

In the libstdc++ build directory (e.g. x86_64-pc-linux-gnu/libstdc++-v3) run make check to run the tests, adding -j8 or a suitable option for your number of cores.

See https://gcc.gnu.org/onlinedocs/libstdc++/manual/test.html#test.run for testing docs.

Once you're happy with the results see https://gcc.gnu.org/contribute.html for patch submission.