From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lxmtout1.gsi.de (lxmtout1.gsi.de [140.181.3.111]) by sourceware.org (Postfix) with ESMTPS id 583FC385742A; Tue, 22 Jun 2021 16:03:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 583FC385742A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gsi.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gsi.de Received: from localhost (localhost [127.0.0.1]) by lxmtout1.gsi.de (Postfix) with ESMTP id 320CC2050D05; Tue, 22 Jun 2021 18:03:10 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at lxmtout1.gsi.de Received: from lxmtout1.gsi.de ([127.0.0.1]) by localhost (lxmtout1.gsi.de [127.0.0.1]) (amavisd-new, port 10024) with LMTP id YrSXi0wt2Y3j; Tue, 22 Jun 2021 18:03:10 +0200 (CEST) Received: from srvex3.campus.gsi.de (unknown [10.10.4.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by lxmtout1.gsi.de (Postfix) with ESMTPS id 17B372050D02; Tue, 22 Jun 2021 18:03:10 +0200 (CEST) Received: from minbar.localnet (140.181.3.12) by srvex3.campus.gsi.de (10.10.4.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2242.10; Tue, 22 Jun 2021 18:03:09 +0200 From: Matthias Kretz To: Jonathan Wakely CC: libstdc++ , gcc Patches Subject: Re: [PATCH v2] libstdc++: Improve std::lock algorithm Date: Tue, 22 Jun 2021 18:03:09 +0200 Message-ID: <9108135.T7Z3S40VBb@minbar> Organization: GSI Helmholtz Centre for Heavy Ion Research In-Reply-To: References: <14287489.dOz4zN4RiP@minbar> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" X-Originating-IP: [140.181.3.12] X-ClientProxiedBy: srvex1.Campus.gsi.de (10.10.4.11) To srvex3.campus.gsi.de (10.10.4.16) X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00, BODY_8BITS, KAM_DMARC_STATUS, SPF_HELO_PASS, 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: 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: Tue, 22 Jun 2021 16:03:14 -0000 On Dienstag, 22. Juni 2021 17:20:41 CEST Jonathan Wakely wrote: > On Tue, 22 Jun 2021 at 14:21, Matthias Kretz wrote: > > This does a try_lock on all lockabes even if any of them fails. I think > > that's > > not only more expensive but also non-conforming. I think you need to de= fer > > locking and then loop from beginning to end to break the loop on the fi= rst > > unsuccessful try_lock. >=20 > Oops, good point. I'll add a test for that too. Here's the fixed code: >=20 > template > inline int > __try_lock_impl(_L0& __l0, _Lockables&... __lockables) > { > #if __cplusplus >=3D 201703L > if constexpr ((is_same_v<_L0, _Lockables> && ...)) > { > constexpr int _Np =3D 1 + sizeof...(_Lockables); > unique_lock<_L0> __locks[_Np] =3D { > {__l0, defer_lock}, {__lockables, defer_lock}... > }; > for (int __i =3D 0; __i < _Np; ++__i) I thought coding style requires a { here? > if (!__locks[__i].try_lock()) > { > const int __failed =3D __i; > while (__i--) > __locks[__i].unlock(); > return __i; You meant `return __failed`? > } > for (auto& __l : __locks) > __l.release(); > return -1; > } > else > #endif >=20 > > [...] > > Yes, if only we had a wrapping integer type that wraps at an arbitrary = N. > > Like > >=20 > > unsigned int but with parameter, like: > > for (__wrapping_uint<_Np> __k =3D __idx; __k !=3D __first; --__k) > > =20 > > __locks[__k - 1].unlock(); > >=20 > > This is the loop I wanted to write, except --__k is simpler to write and > > __k - > > 1 would also wrap around to _Np - 1 for __k =3D=3D 0. But if this is th= e only > > place it's not important enough to abstract. >=20 > We might be able to use __wrapping_uint in std::seed_seq::generate too, a= nd > maybe some other places in . But we can add that later if we deci= de > it's worth it. OK. > > I also considered moving it down here. Makes sense unless you want to c= all > > __detail::__lock_impl from other functions. And if we want to make it w= ork > > for > > pre-C++11 we could do > >=20 > > using __homogeneous > > =20 > > =3D __and_, is_same<_L1, _L3>...>; > > =20 > > int __i =3D 0; > > __detail::__lock_impl(__homogeneous(), __i, 0, __l1, __l2, __l3...); >=20 > We don't need tag dispatching, we could just do: >=20 > if _GLIBCXX17_CONSTEXPR (homogeneous::value) > ... > else > ... >=20 > because both branches are valid for the homogeneous case, i.e. we aren't > using if-constexpr to avoid invalid instantiations. But for the inhomogeneous case the homogeneous code is invalid (initializat= ion=20 of C-array of unique_lock<_L1>). > But given that the default -std option is gnu++17 now, I'm OK with the > iterative version only being used for C++17. =46air enough. =2D-=20 =E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2= =94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94= =80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80= =E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2= =94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94= =80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80= =E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2= =94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94= =80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80 Dr. Matthias Kretz https://mattkretz.github.io GSI Helmholtz Centre for Heavy Ion Research https://gsi.de std::experimental::simd https://github.com/VcDevel/std-simd =E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2= =94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94= =80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80= =E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2= =94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94= =80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80= =E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2= =94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94= =80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80=E2=94=80