From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by sourceware.org (Postfix) with ESMTPS id 2057F3853560 for ; Fri, 24 Jun 2022 07:23:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2057F3853560 Received: by mail-wr1-x42f.google.com with SMTP id q9so1898642wrd.8 for ; Fri, 24 Jun 2022 00:23:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=XyLUiAS4HBINCpn6J9KGF7FMwfT0OjXEKkOXvPC6msY=; b=ZUOtmfkRvcjDW2samfIcBghpzwXWzOy9jnqC2z+wsjv95RTz+ia4ftN60T9lTnRUwd qBefE4pxullZiDi60nr6Gvr6KVV914vsuS7LXWsYyXcf0sxe22sXPbw9hgT0EeP3nKDH cMuMn4kDvipQLj0aPwydCvUo09qpKzWJsOo0MjBO5GfIs4EJByNlpo4hN/IWZf4AlaGn SdDQaJrT7JCZ0p4tQLH+QZYv7kAjo4TGwD2aiZpE/VNqBuejVmQdKmeD8lx8Hlc9PWgK GSI1G/3gr4pOGrIErHIsqDv90UeoKauGl7v6BidIRB841BEK5iI7Ex0cc7B7H3qDiAb4 vcFw== X-Gm-Message-State: AJIora/eRZZ9xp6PG/XuTuNAC3jin03xRAxJsoB4KVCyICLQvvAKzfZ/ CxSln5ykADWVRh8uWA/fA34OU/toA/YK+FdsbTo= X-Google-Smtp-Source: AGRyM1tOhoONPkLSTldFD9Cb4lqpgKFiBzzjX+5qA/WOjtm1B2el6wkxBmQLerhdZNTgio6ST03LORLrHyLAq6OVk7o= X-Received: by 2002:a5d:4d8f:0:b0:21b:a038:ad7e with SMTP id b15-20020a5d4d8f000000b0021ba038ad7emr11527712wru.593.1656055388671; Fri, 24 Jun 2022 00:23:08 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Jonathan Wakely Date: Fri, 24 Jun 2022 08:22:56 +0100 Message-ID: Subject: Re: First-time contributor -- speeding up `std::unique_ptr` compilation To: mail@vittorioromeo.com Cc: "libstdc++" X-Spam-Status: No, score=-1.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, HTML_MESSAGE, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Jun 2022 07:23:12 -0000 On Fri, 24 Jun 2022, 07:20 Jonathan Wakely, wrote: > > > On Fri, 24 Jun 2022, 01:19 , 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 `` 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 '' >> 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 . 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. > >