From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52d.google.com (mail-ed1-x52d.google.com [IPv6:2a00:1450:4864:20::52d]) by sourceware.org (Postfix) with ESMTPS id 7FEE83858D35 for ; Wed, 4 Jan 2023 18:21:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7FEE83858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-ed1-x52d.google.com with SMTP id g1so35527819edj.8 for ; Wed, 04 Jan 2023 10:21:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:from:content-language:references:to:subject:user-agent :mime-version:date:message-id:from:to:cc:subject:date:message-id :reply-to; bh=FhPqye8+EMt/s2xcLT/WlSrU/f4i5pel9JBES2BUFQY=; b=EyluOKOqL81yD2irKq6WOzV52pgB/Mte8MH1DIrCFP65XSqCYKbIR50aXMGWL7qdrR L8xfYIIazgPhpw2/Xu5MAqRsBvSbQHwgebKAfOIu4yfHqWnrs+4wFXQl5Ng1eg+KaEYu zo/Ybc2GTW7ifjOn2IMrufKNRf5orMpO9BYH7aGrY1vamXb+RTyt0pyoE14/lUdY9YQA cXT2n8DAeiFzIinC6PwLIfo3eCO8WX6ogmm/BuP1J+0Wt7Sy1i2Pj5CQiSkWv9C2tvsv PqVRlT1pbDvtFLNG1OJWX+ziZ8nB3l6E7CVU0NRhlUSSYsCNH+SYfahUaPvTWKCmbhQY +aPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:from:content-language:references:to:subject:user-agent :mime-version:date:message-id:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=FhPqye8+EMt/s2xcLT/WlSrU/f4i5pel9JBES2BUFQY=; b=jLcgOVIXZnYsVd1OqV/Cd6Pm4HzfVBbU+taFoKgmVtqupbaLoFoEuyp28MxLCPoGHH sqGfW3S8PKlWaMKwOZknx30y0A1r6MUpQCFjsfyK0UGpAvfUbUDgD2eTVUWSj5Fwla5c 4ofQ9bgJliMWi5NJUmgSYDlPeCq4PB0Dg0r5gcf5R4uq+Sqhnll5IyN+ykmqs/cc8sQX fmNk++IbtrrZ3XFpm6+Mdh3Cqk2A0fVhYkLaqk9/8DizoGXbkXC5pvVDceIvL8Rk1n+l y9IghwDqa1HOGq8lkbgEro3io3aUwTdz2ZLvBNVDPfOh6hTxldtXoY8JvI8cuIgmpkpM kosg== X-Gm-Message-State: AFqh2ko3LCTGgo+vTbPPsO8JcvL397CGSDgRiBUwEB652hlg1Fyn067I YdhSYAWaHKdyVBWwrkpLKmI= X-Google-Smtp-Source: AMrXdXsZqAAvzbxC8srb+0qms8OUgOKHcyvGyp9kkAtaFY0iKP+Sy2XD7mb0I2ferNBdMy1fkzYJoQ== X-Received: by 2002:a05:6402:380d:b0:47e:eaae:9a5b with SMTP id es13-20020a056402380d00b0047eeaae9a5bmr40524724edb.42.1672856459144; Wed, 04 Jan 2023 10:20:59 -0800 (PST) Received: from [10.40.1.219] ([109.190.253.11]) by smtp.googlemail.com with ESMTPSA id u22-20020aa7d996000000b0047021294426sm15088163eds.90.2023.01.04.10.20.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 04 Jan 2023 10:20:57 -0800 (PST) Content-Type: multipart/mixed; boundary="------------aBlsKg3drF1wYG3d0uvwkce5" Message-ID: <52e5d904-da8a-14f1-6704-53f89dbd2d69@gmail.com> Date: Wed, 4 Jan 2023 19:20:43 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: Re: minor optimization bug in basic_string move assignment To: waffl3x , "libstdc++@gcc.gnu.org" References: Content-Language: fr, en-US From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= In-Reply-To: X-Spam-Status: No, score=-10.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This is a multi-part message in MIME format. --------------aBlsKg3drF1wYG3d0uvwkce5 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 04/01/23 00:11, waffl3x via Libstdc++ wrote: > Example: https://godbolt.org/z/sKhGqG1qK > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/basic_string.h;hb=HEAD#l880 > When move assigning to a basic_string, the allocated memory of the moved into string is stored into the source string instead of deallocating it, a good optimization when everything is compatible. However in the case of a stateful allocator (is_always_true() evaluating as false) this optimization is never taken. Unless there is some reason I can't think of that makes equal stateful allocators incompatible here, I believe the if statement on line 880 of basic_string.h should also compare the equality of each strings allocator. The first condition in the function seems to indicate to me that this scenario was being considered and just forgotten about, as the memory doesn't get deallocated immediately if the two allocators are equal. I'll note that because of how everything is handled, this doesn't result in a leak so this bug is still only a minor missed optimization. > > mailto:libstdc++@gcc.gnu.org Hmmm, I don't know, at least it is not as simple as you present it. You cannot add a check on allocator equality as you are proposing because it is too late. __str allocator might have already been propagated to *this on the previous call to std::__alloc_on_move. Note that current check is done only if !_Alloc_traits::_S_propagate_on_move_assign(). This patch might do the job but I wonder if equal allocators can become un-equal after the propagate-on-move-assignment ? --------------aBlsKg3drF1wYG3d0uvwkce5 Content-Type: text/x-patch; charset=UTF-8; name="basic_string.h.patch" Content-Disposition: attachment; filename="basic_string.h.patch" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL2xpYnN0ZGMrKy12My9pbmNsdWRlL2JpdHMvYmFzaWNfc3RyaW5nLmgg Yi9saWJzdGRjKystdjMvaW5jbHVkZS9iaXRzL2Jhc2ljX3N0cmluZy5oCmluZGV4IDEwOTkx NTY1M2FmLi4yNjI3NjMxZTEwMiAxMDA2NDQKLS0tIGEvbGlic3RkYysrLXYzL2luY2x1ZGUv Yml0cy9iYXNpY19zdHJpbmcuaAorKysgYi9saWJzdGRjKystdjMvaW5jbHVkZS9iaXRzL2Jh c2ljX3N0cmluZy5oCkBAIC04NDQsOSArODQ0LDEwIEBAIF9HTElCQ1hYX0JFR0lOX05BTUVT UEFDRV9DWFgxMQogICAgICAgb3BlcmF0b3I9KGJhc2ljX3N0cmluZyYmIF9fc3RyKQogICAg ICAgbm9leGNlcHQoX0FsbG9jX3RyYWl0czo6X1Nfbm90aHJvd19tb3ZlKCkpCiAgICAgICB7 CisJY29uc3QgYm9vbCBfX2VxdWFsX2FsbG9jcyA9IF9BbGxvY190cmFpdHM6Ol9TX2Fsd2F5 c19lcXVhbCgpCisJICAgIHx8IF9NX2dldF9hbGxvY2F0b3IoKSA9PSBfX3N0ci5fTV9nZXRf YWxsb2NhdG9yKCkKIAlpZiAoIV9NX2lzX2xvY2FsKCkgJiYgX0FsbG9jX3RyYWl0czo6X1Nf cHJvcGFnYXRlX29uX21vdmVfYXNzaWduKCkKLQkgICAgJiYgIV9BbGxvY190cmFpdHM6Ol9T X2Fsd2F5c19lcXVhbCgpCi0JICAgICYmIF9NX2dldF9hbGxvY2F0b3IoKSAhPSBfX3N0ci5f TV9nZXRfYWxsb2NhdG9yKCkpCisJICAgICYmICFfX2VxdWFsX2FsbG9jcykKIAkgIHsKIAkg ICAgLy8gRGVzdHJveSBleGlzdGluZyBzdG9yYWdlIGJlZm9yZSByZXBsYWNpbmcgYWxsb2Nh dG9yLgogCSAgICBfTV9kZXN0cm95KF9NX2FsbG9jYXRlZF9jYXBhY2l0eSk7CkBAIC04Njgs MTYgKzg2OSwxNCBAQCBfR0xJQkNYWF9CRUdJTl9OQU1FU1BBQ0VfQ1hYMTEKIAkJX01fc2V0 X2xlbmd0aChfX3N0ci5zaXplKCkpOwogCSAgICAgIH0KIAkgIH0KLQllbHNlIGlmIChfQWxs b2NfdHJhaXRzOjpfU19wcm9wYWdhdGVfb25fbW92ZV9hc3NpZ24oKQotCSAgICB8fCBfQWxs b2NfdHJhaXRzOjpfU19hbHdheXNfZXF1YWwoKQotCSAgICB8fCBfTV9nZXRfYWxsb2NhdG9y KCkgPT0gX19zdHIuX01fZ2V0X2FsbG9jYXRvcigpKQorCWVsc2UgaWYgKF9BbGxvY190cmFp dHM6Ol9TX3Byb3BhZ2F0ZV9vbl9tb3ZlX2Fzc2lnbigpIHx8IF9fZXF1YWxfYWxsb2NzKQog CSAgewogCSAgICAvLyBKdXN0IG1vdmUgdGhlIGFsbG9jYXRlZCBwb2ludGVyLCBvdXIgYWxs b2NhdG9yIGNhbiBmcmVlIGl0LgogCSAgICBwb2ludGVyIF9fZGF0YSA9IG51bGxwdHI7CiAJ ICAgIHNpemVfdHlwZSBfX2NhcGFjaXR5OwogCSAgICBpZiAoIV9NX2lzX2xvY2FsKCkpCiAJ ICAgICAgewotCQlpZiAoX0FsbG9jX3RyYWl0czo6X1NfYWx3YXlzX2VxdWFsKCkpCisJCWlm IChfX2VxdWFsX2FsbG9jcykKIAkJICB7CiAJCSAgICAvLyBfX3N0ciBjYW4gcmV1c2Ugb3Vy IGV4aXN0aW5nIHN0b3JhZ2UuCiAJCSAgICBfX2RhdGEgPSBfTV9kYXRhKCk7Cg== --------------aBlsKg3drF1wYG3d0uvwkce5--