From patchwork Thu Aug 9 18:41:43 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Fran=C3=A7ois_Dumont?= X-Patchwork-Id: 955759 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-483474-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="juJHNg5a"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="TFD+wPTs"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41mcYh4SVNz9s1x for ; Fri, 10 Aug 2018 04:42:14 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:message-id:date:mime-version:content-type; q=dns; s= default; b=tVFJ4cDFqEAy7kShJ4QuLpb1vrcxJQQ3EdiHK8e+ZnhnoZ3/WF+oN VsbvczAaxphoAg42agM8Msu/Nwxp49T9vVD+B87nsGKWJ147B+Dq84OBHDC5QW9h Q2w/pC6ZfGlumFLsvEMen9ly2W40hJOkyUzyhuh+CJS1OgR4COJdJs= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:message-id:date:mime-version:content-type; s= default; bh=VGS72Uh9Saic+b4Rr5wrWXViHoE=; b=juJHNg5aiL1WwojT5oT8 R9Gufb7gvWCDTuzvVMbbt2vuwXH7HQ5ow7wlwIlX4d7G7Xn7ZWtAE1kpo9vPUnYc eoF1WzAO/ohTbtXFEHCIBmBlGLO2T0NI61dyQeB3rsKmaVF0vgUOZcgsFfDcc2Ho veSJJH1e3ZyCByVHOwlKw/E= Received: (qmail 49718 invoked by alias); 9 Aug 2018 18:41:59 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 49700 invoked by uid 89); 9 Aug 2018 18:41:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?iso-8859-1?q?No=2C_score=3D-26=2E9_required=3D5=2E?= =?iso-8859-1?q?0_tests=BAYES_00=2CFREEMAIL_FROM=2CGIT_PATCH_0=2CGI?= =?iso-8859-1?q?T_PATCH_1=2CGIT_PATCH_2=2CGIT_PATCH_3=2CRCVD_IN_DNS?= =?iso-8859-1?q?WL_NONE=2CSPF_PASS=2CTIME_LIMIT_EXCEEDED_autolearn?= =?iso-8859-1?q?=3Dunavailable_version=3D3=2E3=2E2_spammy=3Dfranois?= =?iso-8859-1?q?=2C_initializing=2C_fran=C3=A7ois=2C_singular?= X-HELO: mail-wm0-f48.google.com Received: from mail-wm0-f48.google.com (HELO mail-wm0-f48.google.com) (74.125.82.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 09 Aug 2018 18:41:48 +0000 Received: by mail-wm0-f48.google.com with SMTP id t25-v6so1273624wmi.3; Thu, 09 Aug 2018 11:41:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:subject:to:message-id:date:user-agent:mime-version :content-language; bh=Le0KLpyGD3kzGkl4NciND4x3v143hAofnC6Vr4awBoU=; b=TFD+wPTszBV21OwEEDgFnJtIYKarPhF7x7DaATQiIEC9MLOIqz5jEgpQLtpN+6SZTM zlrh6r49qDV99a+5Csl8Tof6T5O5PGmjQR0XgpxNL7ufdh34K5ZHHsJpcLt0JD0vg3UO nSYuMBYm0/O1E3x1TBYVQhYYGtz68cRMZl9CSgcZKtCXwJ+G8TmtUvqcBzdquUf+e+eS jxQzamHjHjNZ/17e31Uxq4Jw3ozNlR5KmQe9ft9MiIfAr+rWjJvL+y27PVSxkTnOAzTa /R4Yqby0ApOTzgLt1PHBJ/sudIL/bBz5+/rP7/6JMXfn4v7GaEfgLVAU3X2AOU0F3X2Y Ispw== Received: from [192.168.0.23] (arf62-1-82-237-250-248.fbx.proxad.net. [82.237.250.248]) by smtp.googlemail.com with ESMTPSA id q188-v6sm15716717wmd.36.2018.08.09.11.41.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Aug 2018 11:41:45 -0700 (PDT) From: =?utf-8?q?Fran=C3=A7ois_Dumont?= Subject: Improve safe iterator move semantic To: "libstdc++@gcc.gnu.org" , gcc-patches Message-ID: <2058d369-69a9-e4ad-a4d0-147277bd766a@gmail.com> Date: Thu, 9 Aug 2018 20:41:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 Here is a patch to improve Debug mode safe iterator move semantic.     To summarize where we used to have N mutex locks we now have N - 1. For instance move constructor used to lock mutex twice, now it only does it once. Note that move constructor or move assignment operator are currently more expensive than their copy counterparts !     I did my best to avoid new symbols but I still require 1 to be added: _Safe_local_iterator::_M_attach_single. We will now make use of the already exported _Safe_iterator::_M_attach_single symbol.     Tested under Linux x86_64, Debug mode. Ok to commit ? François diff --git a/libstdc++-v3/config/abi/pre/gnu.ver b/libstdc++-v3/config/abi/pre/gnu.ver index 593783d..1143d9a 100644 --- a/libstdc++-v3/config/abi/pre/gnu.ver +++ b/libstdc++-v3/config/abi/pre/gnu.ver @@ -2046,6 +2046,7 @@ GLIBCXX_3.4.26 { _ZNSt3pmr25monotonic_buffer_resource13_M_new_bufferE[jmy][jmy]; _ZNSt3pmr25monotonic_buffer_resource18_M_release_buffersEv; + _ZN11__gnu_debug25_Safe_local_iterator_base16_M_detach_singleEv; } GLIBCXX_3.4.25; # Symbols in the support library (libsupc++) have their own tag. diff --git a/libstdc++-v3/include/debug/safe_base.h b/libstdc++-v3/include/debug/safe_base.h index f58a78f..c276a18 100644 --- a/libstdc++-v3/include/debug/safe_base.h +++ b/libstdc++-v3/include/debug/safe_base.h @@ -97,6 +97,13 @@ namespace __gnu_debug : _M_sequence(0), _M_version(0), _M_prior(0), _M_next(0) { this->_M_attach(__x._M_sequence, __constant); } +#if __cplusplus >= 201103L + _Safe_iterator_base(_Safe_iterator_base&&) = default; + + _Safe_iterator_base& + operator=(_Safe_iterator_base&&) = default; +#endif + ~_Safe_iterator_base() { this->_M_detach(); } /** For use in _Safe_iterator. */ @@ -121,6 +128,12 @@ namespace __gnu_debug void _M_detach(); +#if __cplusplus >= 201103L + /** Attaches this iterator at __x place. */ + void + _M_move_to(_Safe_iterator_base* __x, bool __constant); +#endif + public: /** Likewise, but not thread-safe. */ void @@ -148,11 +161,12 @@ namespace __gnu_debug /** Reset all member variables */ void - _M_reset() throw (); + _M_reset() _GLIBCXX_USE_NOEXCEPT + { __builtin_memset(this, 0, sizeof(_Safe_iterator_base)); } /** Unlink itself */ void - _M_unlink() throw () + _M_unlink() _GLIBCXX_USE_NOEXCEPT { if (_M_prior) _M_prior->_M_next = _M_next; @@ -273,6 +287,28 @@ namespace __gnu_debug void _M_detach_single(_Safe_iterator_base* __it) throw (); }; + +#if __cplusplus >= 201103L + inline void + _Safe_iterator_base::_M_move_to(_Safe_iterator_base* __x, bool __constant) + { + __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); + if (_M_prior) + _M_prior->_M_next = this; + else + { + // No prior, we are at the beginning of the linked list. + auto& __its = __constant + ? _M_sequence->_M_const_iterators : _M_sequence->_M_iterators; + if (__its == __x) + __its = this; + } + + if (_M_next) + _M_next->_M_prior = this; + } +#endif + } // namespace __gnu_debug #endif diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index b8256fc..4b5773f 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -151,17 +151,20 @@ namespace __gnu_debug * @post __x is singular and unattached */ _Safe_iterator(_Safe_iterator&& __x) noexcept - : _Iter_base() + : _Iter_base(__x), _Safe_base(std::move(__x)) { _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() || __x.base() == _Iterator(), _M_message(__msg_init_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); - _Safe_sequence_base* __seq = __x._M_sequence; - __x._M_detach(); - std::swap(base(), __x.base()); - _M_attach(__seq); + if (__x._M_sequence) + { + _M_move_to(&__x, _S_constant()); + + __x._M_reset(); + __x.base() = _Iter_base(); + } } #endif @@ -238,16 +241,22 @@ namespace __gnu_debug { __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); base() = __x.base(); - _M_version = __x._M_sequence->_M_version; + _M_version = __x._M_version; + __x._M_detach_single(); } else { _M_detach(); base() = __x.base(); - _M_attach(__x._M_sequence); + + if (__x._M_sequence) + { + _Safe_base::operator=(std::move(__x)); + _M_move_to(&__x, _S_constant()); + __x._M_reset(); + } } - __x._M_detach(); __x.base() = _Iterator(); return *this; } diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h index f9597a6..2dd9a19 100644 --- a/libstdc++-v3/include/debug/safe_local_iterator.h +++ b/libstdc++-v3/include/debug/safe_local_iterator.h @@ -112,17 +112,20 @@ namespace __gnu_debug * @post __x is singular and unattached */ _Safe_local_iterator(_Safe_local_iterator&& __x) noexcept - : _Iter_base() + : _Iter_base(__x), _Safe_base(std::move(__x)) { _GLIBCXX_DEBUG_VERIFY(!__x._M_singular() || __x.base() == _Iterator(), _M_message(__msg_init_copy_singular) ._M_iterator(*this, "this") ._M_iterator(__x, "other")); - auto __cont = __x._M_sequence; - __x._M_detach(); - std::swap(base(), __x.base()); - _M_attach(__cont); + if (__x._M_sequence) + { + _M_move_to(&__x, _S_constant()); + + __x._M_reset(); + __x.base() = _Iter_base(); + } } /** @@ -198,16 +201,22 @@ namespace __gnu_debug { __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); base() = __x.base(); - _M_version = __x._M_sequence->_M_version; + _M_version = __x._M_version; + __x._M_detach_single(); } else { _M_detach(); base() = __x.base(); - _M_attach(__x._M_sequence); + + if (__x._M_sequence) + { + _Safe_base::operator=(std::move(__x)); + _M_move_to(&__x, _S_constant()); + __x._M_reset(); + } } - __x._M_detach(); __x.base() = _Iterator(); return *this; } diff --git a/libstdc++-v3/include/debug/safe_unordered_base.h b/libstdc++-v3/include/debug/safe_unordered_base.h index 5dd0e0e..340aea6 100644 --- a/libstdc++-v3/include/debug/safe_unordered_base.h +++ b/libstdc++-v3/include/debug/safe_unordered_base.h @@ -51,8 +51,9 @@ namespace __gnu_debug { protected: /** Initializes the iterator and makes it singular. */ - _Safe_local_iterator_base() - { } + _Safe_local_iterator_base() = default; + + _Safe_local_iterator_base(_Safe_local_iterator_base&&) = default; /** Initialize the iterator to reference the container pointed to * by @p __seq. @p __constant is true when we are initializing a @@ -62,17 +63,20 @@ namespace __gnu_debug * be nonsingular. */ _Safe_local_iterator_base(const _Safe_sequence_base* __seq, bool __constant) - { this->_M_attach(const_cast<_Safe_sequence_base*>(__seq), __constant); } + { _M_attach(const_cast<_Safe_sequence_base*>(__seq), __constant); } /** Initializes the iterator to reference the same container that @p __x does. @p __constant is true if this is a constant iterator, and false if it is mutable. */ _Safe_local_iterator_base(const _Safe_local_iterator_base& __x, bool __constant) - { this->_M_attach(__x._M_sequence, __constant); } + { _M_attach(__x._M_sequence, __constant); } ~_Safe_local_iterator_base() { this->_M_detach(); } + _Safe_local_iterator_base& + operator=(_Safe_local_iterator_base&&) = default; + _Safe_unordered_container_base* _M_get_container() const noexcept; @@ -97,6 +101,10 @@ namespace __gnu_debug /** Likewise, but not thread-safe. */ void _M_detach_single() throw (); + + /** Attaches this iterator at __x place. */ + void + _M_move_to(_Safe_iterator_base* __x, bool __constant); }; /** @@ -180,6 +188,31 @@ namespace __gnu_debug void _M_detach_local_single(_Safe_iterator_base* __it) throw (); }; + + inline _Safe_unordered_container_base* + _Safe_local_iterator_base::_M_get_container() const noexcept + { return static_cast<_Safe_unordered_container_base*>(_M_sequence); } + + inline void + _Safe_local_iterator_base::_M_move_to(_Safe_iterator_base* __x, + bool __constant) + { + __gnu_cxx::__scoped_lock __l(this->_M_get_mutex()); + if (_M_prior) + _M_prior->_M_next = this; + else + { + // No prior, we are at the beginning of the linked list. + auto& __its = __constant + ? _M_get_container()->_M_const_local_iterators + : _M_get_container()->_M_local_iterators; + if (__its == __x) + __its = this; + } + + if (_M_next) + _M_next->_M_prior = this; + } } // namespace __gnu_debug #endif diff --git a/libstdc++-v3/src/c++11/debug.cc b/libstdc++-v3/src/c++11/debug.cc index 3cae3db..8edbfc0 100644 --- a/libstdc++-v3/src/c++11/debug.cc +++ b/libstdc++-v3/src/c++11/debug.cc @@ -401,16 +401,6 @@ namespace __gnu_debug } } - void - _Safe_iterator_base:: - _M_reset() throw () - { - _M_sequence = 0; - _M_version = 0; - _M_prior = 0; - _M_next = 0; - } - bool _Safe_iterator_base:: _M_singular() const throw () @@ -429,11 +419,6 @@ namespace __gnu_debug _M_get_mutex() throw () { return _M_sequence->_M_get_mutex(); } - _Safe_unordered_container_base* - _Safe_local_iterator_base:: - _M_get_container() const noexcept - { return static_cast<_Safe_unordered_container_base*>(_M_sequence); } - void _Safe_local_iterator_base:: _M_attach(_Safe_sequence_base* __cont, bool __constant)