From patchwork Mon Oct 19 17:16:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 1384434 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gcc.gnu.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=jLpsWrbz; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CFNhv3ynrz9sTK for ; Tue, 20 Oct 2020 04:16:47 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 501B9388EC0D; Mon, 19 Oct 2020 17:16:35 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 501B9388EC0D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1603127795; bh=QmTWymvrXH7wLPWF+YATawWvluuByeFPBCspb40x43M=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=jLpsWrbz2HASYkUV5C2rZoG14zdITWgNjVxj/i+38TOzF/2WSem8o9F3cId6MfJ7T gm+nZGh7jBdWEiuUt/qf66HdsFaHwavBt1DllahMKmeENXpce7pANV2eOmTQjBcRSq ODmpkXFvu3h7l/JrOqvucKxRAAq5xHZnZ0rDxwZ0= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by sourceware.org (Postfix) with ESMTP id 5F7EE3854826 for ; Mon, 19 Oct 2020 17:16:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5F7EE3854826 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-540-CGGu1zwWPOWxvVuot70g3Q-1; Mon, 19 Oct 2020 13:16:18 -0400 X-MC-Unique: CGGu1zwWPOWxvVuot70g3Q-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 4571E1006C86; Mon, 19 Oct 2020 17:16:17 +0000 (UTC) Received: from localhost (unknown [10.33.36.242]) by smtp.corp.redhat.com (Postfix) with ESMTP id CA26F6EF41; Mon, 19 Oct 2020 17:16:16 +0000 (UTC) Date: Mon, 19 Oct 2020 18:16:15 +0100 To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Optimize container node-handle type for size Message-ID: <20201019171615.GA830484@redhat.com> MIME-Version: 1.0 X-Clacks-Overhead: GNU Terry Pratchett X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-14.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jonathan Wakely via Gcc-patches From: Jonathan Wakely Reply-To: Jonathan Wakely Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" The use of std::optional in _Node_handle makes the node handle types for associative and unordered containers larger than necessary. It also greatly increases the amount of code included, as is quite large. The boolean flag that records whether the std::optional contains a value is redundant, because the _Node_handle::_M_ptr member provides the same information. If the node handle has a non-null pointer it also has an allocator, and not otherwise. By replacing std::optional with a custom union type (and using _M_ptr to tell which union member is active) all node handle sizes can be reduced by sizeof(allocator_type::pointer). This makes the node handle types incompatible with previous releases, so must be done before the C++17 ABI is fixed for GCC 11. libstdc++-v3/ChangeLog: * include/bits/node_handle.h (_Node_handle_common): Replace std::optional with custom type. * testsuite/20_util/variant/exception_safety.cc: Add missing header include. Tested powerpc64le-linux. Committed to trunk. commit aa6d2be1e3455a5f0818d5bd6a44b15a55a39df5 Author: Jonathan Wakely Date: Mon Oct 19 17:56:54 2020 libstdc++: Optimize container node-handle type for size The use of std::optional in _Node_handle makes the node handle types for associative and unordered containers larger than necessary. It also greatly increases the amount of code included, as is quite large. The boolean flag that records whether the std::optional contains a value is redundant, because the _Node_handle::_M_ptr member provides the same information. If the node handle has a non-null pointer it also has an allocator, and not otherwise. By replacing std::optional with a custom union type (and using _M_ptr to tell which union member is active) all node handle sizes can be reduced by sizeof(allocator_type::pointer). This makes the node handle types incompatible with previous releases, so must be done before the C++17 ABI is fixed for GCC 11. libstdc++-v3/ChangeLog: * include/bits/node_handle.h (_Node_handle_common): Replace std::optional with custom type. * testsuite/20_util/variant/exception_safety.cc: Add missing header include. diff --git a/libstdc++-v3/include/bits/node_handle.h b/libstdc++-v3/include/bits/node_handle.h index fc96937665a..82b702426d1 100644 --- a/libstdc++-v3/include/bits/node_handle.h +++ b/libstdc++-v3/include/bits/node_handle.h @@ -33,10 +33,10 @@ #pragma GCC system_header -#if __cplusplus > 201402L +#if __cplusplus >= 201703L # define __cpp_lib_node_extract 201606 -#include +#include #include #include @@ -57,7 +57,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION get_allocator() const noexcept { __glibcxx_assert(!this->empty()); - return allocator_type(*_M_alloc); + return allocator_type(_M_alloc._M_alloc); } explicit operator bool() const noexcept { return _M_ptr != nullptr; } @@ -65,76 +65,151 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION [[nodiscard]] bool empty() const noexcept { return _M_ptr == nullptr; } protected: - constexpr _Node_handle_common() noexcept : _M_ptr(), _M_alloc() {} + constexpr _Node_handle_common() noexcept : _M_ptr() { } - ~_Node_handle_common() { _M_destroy(); } + ~_Node_handle_common() + { + if (!empty()) + _M_reset(); + } _Node_handle_common(_Node_handle_common&& __nh) noexcept - : _M_ptr(__nh._M_ptr), _M_alloc(std::move(__nh._M_alloc)) + : _M_ptr(__nh._M_ptr) { - __nh._M_ptr = nullptr; - __nh._M_alloc = nullopt; + if (_M_ptr) + _M_move(std::move(__nh)); } _Node_handle_common& operator=(_Node_handle_common&& __nh) noexcept { - _M_destroy(); - _M_ptr = __nh._M_ptr; - if constexpr (is_move_assignable_v<_NodeAlloc>) + if (empty()) { - if (_AllocTraits::propagate_on_container_move_assignment::value - || !this->_M_alloc) - this->_M_alloc = std::move(__nh._M_alloc); - else - { - __glibcxx_assert(this->_M_alloc == __nh._M_alloc); - } + if (!__nh.empty()) + _M_move(std::move(__nh)); } + else if (__nh.empty()) + _M_reset(); else { - __glibcxx_assert(_M_alloc); + // Free the current node before replacing the allocator. + _AllocTraits::destroy(*_M_alloc, _M_ptr->_M_valptr()); + _AllocTraits::deallocate(*_M_alloc, _M_ptr, 1); + + _M_alloc = __nh._M_alloc.release(); // assigns if POCMA + _M_ptr = __nh._M_ptr; + __nh._M_ptr = nullptr; } - __nh._M_ptr = nullptr; - __nh._M_alloc = nullopt; return *this; } _Node_handle_common(typename _AllocTraits::pointer __ptr, const _NodeAlloc& __alloc) - : _M_ptr(__ptr), _M_alloc(__alloc) { } + : _M_ptr(__ptr), _M_alloc(__alloc) + { + __glibcxx_assert(__ptr != nullptr); + } void _M_swap(_Node_handle_common& __nh) noexcept { - using std::swap; - swap(_M_ptr, __nh._M_ptr); - if (_AllocTraits::propagate_on_container_swap::value - || !_M_alloc || !__nh._M_alloc) - _M_alloc.swap(__nh._M_alloc); + if (empty()) + { + if (!__nh.empty()) + _M_move(std::move(__nh)); + } + else if (__nh.empty()) + __nh._M_move(std::move(*this)); else { - __glibcxx_assert(_M_alloc == __nh._M_alloc); + using std::swap; + swap(_M_ptr, __nh._M_ptr); + _M_alloc.swap(__nh._M_alloc); // swaps if POCS } } private: + // Moves the pointer and allocator from __nh to *this. + // Precondition: empty() && !__nh.empty() + // Postcondition: !empty() && __nh.empty() void - _M_destroy() noexcept + _M_move(_Node_handle_common&& __nh) noexcept { - if (_M_ptr != nullptr) - { - allocator_type __alloc(*_M_alloc); - allocator_traits::destroy(__alloc, - _M_ptr->_M_valptr()); - _AllocTraits::deallocate(*_M_alloc, _M_ptr, 1); - } + ::new (std::__addressof(_M_alloc)) _NodeAlloc(__nh._M_alloc.release()); + _M_ptr = __nh._M_ptr; + __nh._M_ptr = nullptr; + } + + // Deallocates the node, destroys the allocator. + // Precondition: !empty() + // Postcondition: empty() + void + _M_reset() noexcept + { + _NodeAlloc __alloc = _M_alloc.release(); + _AllocTraits::destroy(__alloc, _M_ptr->_M_valptr()); + _AllocTraits::deallocate(__alloc, _M_ptr, 1); + _M_ptr = nullptr; } protected: - typename _AllocTraits::pointer _M_ptr; + typename _AllocTraits::pointer _M_ptr; + private: - optional<_NodeAlloc> _M_alloc; + // A simplified, non-copyable std::optional<_NodeAlloc>. + // Call release() before destruction iff the allocator member is active. + union _Optional_alloc + { + _Optional_alloc() { } + ~_Optional_alloc() { } + + _Optional_alloc(_Optional_alloc&&) = delete; + _Optional_alloc& operator=(_Optional_alloc&&) = delete; + + _Optional_alloc(const _NodeAlloc& __alloc) noexcept + : _M_alloc(__alloc) + { } + + // Precondition: _M_alloc is the active member of the union. + void + operator=(_NodeAlloc&& __alloc) noexcept + { + using _ATr = _AllocTraits; + if constexpr (_ATr::propagate_on_container_move_assignment::value) + _M_alloc = std::move(__alloc); + else if constexpr (!_AllocTraits::is_always_equal::value) + __glibcxx_assert(_M_alloc == __alloc); + } + + // Precondition: _M_alloc is the active member of both unions. + void + swap(_Optional_alloc& __other) noexcept + { + using std::swap; + if constexpr (_AllocTraits::propagate_on_container_swap::value) + swap(_M_alloc, __other._M_alloc); + else if constexpr (!_AllocTraits::is_always_equal::value) + __glibcxx_assert(_M_alloc == __other._M_alloc); + } + + // Precondition: _M_alloc is the active member of the union. + _NodeAlloc& operator*() noexcept { return _M_alloc; } + + // Precondition: _M_alloc is the active member of the union. + _NodeAlloc release() noexcept + { + _NodeAlloc __tmp = std::move(_M_alloc); + _M_alloc.~_NodeAlloc(); + return __tmp; + } + + struct _Empty { }; + + [[__no_unique_address__]] _Empty _M_empty; + [[__no_unique_address__]] _NodeAlloc _M_alloc; + }; + + [[__no_unique_address__]] _Optional_alloc _M_alloc; template diff --git a/libstdc++-v3/testsuite/20_util/variant/exception_safety.cc b/libstdc++-v3/testsuite/20_util/variant/exception_safety.cc index 019b4e4dca4..c96b9212592 100644 --- a/libstdc++-v3/testsuite/20_util/variant/exception_safety.cc +++ b/libstdc++-v3/testsuite/20_util/variant/exception_safety.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include void