From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x534.google.com (mail-ed1-x534.google.com [IPv6:2a00:1450:4864:20::534]) by sourceware.org (Postfix) with ESMTPS id CB0063858D28; Wed, 25 Jan 2023 18:38:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CB0063858D28 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-x534.google.com with SMTP id w11so7474554edv.0; Wed, 25 Jan 2023 10:38:55 -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:cc:to:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=ueafQ1Hurs5FdKF3mlsOnk8q55mznmg/TXoRyMix6iU=; b=bMiHvCSBBal7uNMunyWRQudnrN8/xhdUG6TmqDRQhloLOJJacKVY2bqEoDhdY4ijx+ ntxJICJkvRsn4W42twwglE12QQLuTHgGmhzhNps7oU2tecZ5rB16IUE5JCkFoOcAtpw2 x4titSQ0xF3PYSDc1GHjBe6OnjRuhPCyzjAhRQKEnoyX8vG/3LptCqfpOmGaNIq2cq1z LPqowPUTbXtaFoFlJQFlsOdKGff7e9iA4xBJhQ/MqJGDoAqhimlEDS1hE/ewDliTcohU oODueIf0OvvXvUsGWXYVarapDvLo79ZAeMIoBbW0OIy5NhTPCKCQ6BOe6dJVIciF36R4 /s8A== 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:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=ueafQ1Hurs5FdKF3mlsOnk8q55mznmg/TXoRyMix6iU=; b=p3qnGFm/tzUlZpWg18mn6SexU3ZpdIz9izKtgw4XZ0xufYEGRVA+hLWYLngoGpCS7y Eq81TwizS3Qy+tx/5twYp3DwXBrSFYccZ/0zratO8QYcPiiQg/ekQX9cX492PUKY6riN /rqhlrbvoPk4M0ejPbxJUI/pKqMRuAEttkizeXQzk5Vq9rJkoL9v7pi+Hd3P0iwAe9bB VgpeZwa2tnIivMP+fir5jt0jIxZ1tKZvswU136VTZAE0Pv6+BT2vNRXefj78NBlqtFHw HaCGpseK5+RM6xyN/ICRaEqHoOvmXsZemYVLUvPPSF+11IgRsB4Wgw/8fe3ZnbUNrn51 rGiQ== X-Gm-Message-State: AFqh2kocH/orBFBwWNWz4mL1RjXwU/nKFsPgwK0gXj33Yrna5QXcXwZU 9f1y4tJAcjCru2EyIAS8EIs= X-Google-Smtp-Source: AMrXdXu3UTwy9wPmbWFEhU5MYkwYJ+HcuBPpQtakE123G6GkVrqAsslcRrg6PMbPp56TkFYkjVrJQg== X-Received: by 2002:aa7:cc81:0:b0:465:f6a9:cb7b with SMTP id p1-20020aa7cc81000000b00465f6a9cb7bmr33916028edt.12.1674671934145; Wed, 25 Jan 2023 10:38:54 -0800 (PST) Received: from [10.11.0.210] ([109.190.253.11]) by smtp.googlemail.com with ESMTPSA id et10-20020a056402378a00b00457b5ba968csm2678911edb.27.2023.01.25.10.38.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 25 Jan 2023 10:38:53 -0800 (PST) Content-Type: multipart/mixed; boundary="------------yeY9jAtKabBRzzsu50hjrAje" Message-ID: Date: Wed, 25 Jan 2023 19:38:47 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 Subject: [PATCH] minor optimization bug in basic_string move assignment To: Jonathan Wakely , "libstdc++@gcc.gnu.org" Cc: waffl3x , gcc-patches References: <52e5d904-da8a-14f1-6704-53f89dbd2d69@gmail.com> Content-Language: fr From: =?UTF-8?Q?Fran=c3=a7ois_Dumont?= In-Reply-To: X-Spam-Status: No, score=-8.0 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,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. --------------yeY9jAtKabBRzzsu50hjrAje Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Let's submit a proper patch proposal then. The occasion for me to ask if there is any reason for cow string not being C++11 allocator compliant ? Just lack of interest ? I wanted to consider it to get rid of the __gnu_debug::_Safe_container _IsCxx11AllocatorAware template parameter.     libstdc++: Optimize basic_string move assignment     Since resolution of Issue 2593 [1] we can consider that equal allocators     before the propagate-on-move-assignment operations will still be equal     afterward.     So we can extend the optimization of transfering the storage of the move-to     instance to the move-from one that is currently limited to always equal     allocators.     [1] https://cplusplus.github.io/LWG/issue2593     libstdc++-v3/ChangeLog:             * include/bits/basic_string.h (operator=(basic_string&&)): Transfer move-to             storage to the move-from instance when allocators are equal.             * testsuite/21_strings/basic_string/allocator/char/move_assign.cc (test04):             New test case. Tested under linux x86_64, ok to commit ? François On 17/01/23 20:18, Jonathan Wakely wrote: > On Wed, 4 Jan 2023 at 18:21, François Dumont via Libstdc++ > wrote: >> 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 ? > Since https://cplusplus.github.io/LWG/issue2593 they can't. But I > think when I wrote that code, they could do, which is probably why the > optimization wasn't done. > --------------yeY9jAtKabBRzzsu50hjrAje Content-Type: text/x-patch; charset=UTF-8; name="string_move_assign_optim.patch" Content-Disposition: attachment; filename="string_move_assign_optim.patch" Content-Transfer-Encoding: base64 ZGlmZiAtLWdpdCBhL2xpYnN0ZGMrKy12My9pbmNsdWRlL2JpdHMvYmFzaWNfc3RyaW5nLmgg Yi9saWJzdGRjKystdjMvaW5jbHVkZS9iaXRzL2Jhc2ljX3N0cmluZy5oCmluZGV4IGFhMDE4 MjYyYzk4Li5jODFkYzBkNDI1YSAxMDA2NDQKLS0tIGEvbGlic3RkYysrLXYzL2luY2x1ZGUv Yml0cy9iYXNpY19zdHJpbmcuaAorKysgYi9saWJzdGRjKystdjMvaW5jbHVkZS9iaXRzL2Jh c2ljX3N0cmluZy5oCkBAIC04NDQsOSArODQ0LDEwIEBAIF9HTElCQ1hYX0JFR0lOX05BTUVT UEFDRV9DWFgxMQogICAgICAgb3BlcmF0b3I9KGJhc2ljX3N0cmluZyYmIF9fc3RyKQogICAg ICAgbm9leGNlcHQoX0FsbG9jX3RyYWl0czo6X1Nfbm90aHJvd19tb3ZlKCkpCiAgICAgICB7 CisJY29uc3QgYm9vbCBfX2VxdWFsX2FsbG9jcyA9IF9BbGxvY190cmFpdHM6Ol9TX2Fsd2F5 c19lcXVhbCgpCisJICB8fCBfTV9nZXRfYWxsb2NhdG9yKCkgPT0gX19zdHIuX01fZ2V0X2Fs bG9jYXRvcigpOwogCWlmICghX01faXNfbG9jYWwoKSAmJiBfQWxsb2NfdHJhaXRzOjpfU19w cm9wYWdhdGVfb25fbW92ZV9hc3NpZ24oKQotCSAgICAmJiAhX0FsbG9jX3RyYWl0czo6X1Nf YWx3YXlzX2VxdWFsKCkKLQkgICAgJiYgX01fZ2V0X2FsbG9jYXRvcigpICE9IF9fc3RyLl9N X2dldF9hbGxvY2F0b3IoKSkKKwkgICAgJiYgIV9fZXF1YWxfYWxsb2NzKQogCSAgewogCSAg ICAvLyBEZXN0cm95IGV4aXN0aW5nIHN0b3JhZ2UgYmVmb3JlIHJlcGxhY2luZyBhbGxvY2F0 b3IuCiAJICAgIF9NX2Rlc3Ryb3koX01fYWxsb2NhdGVkX2NhcGFjaXR5KTsKQEAgLTg2OCwx NiArODY5LDE0IEBAIF9HTElCQ1hYX0JFR0lOX05BTUVTUEFDRV9DWFgxMQogCQlfTV9zZXRf bGVuZ3RoKF9fc3RyLnNpemUoKSk7CiAJICAgICAgfQogCSAgfQotCWVsc2UgaWYgKF9BbGxv Y190cmFpdHM6Ol9TX3Byb3BhZ2F0ZV9vbl9tb3ZlX2Fzc2lnbigpCi0JICAgIHx8IF9BbGxv Y190cmFpdHM6Ol9TX2Fsd2F5c19lcXVhbCgpCi0JICAgIHx8IF9NX2dldF9hbGxvY2F0b3Io KSA9PSBfX3N0ci5fTV9nZXRfYWxsb2NhdG9yKCkpCisJZWxzZSBpZiAoX0FsbG9jX3RyYWl0 czo6X1NfcHJvcGFnYXRlX29uX21vdmVfYXNzaWduKCkgfHwgX19lcXVhbF9hbGxvY3MpCiAJ ICB7CiAJICAgIC8vIEp1c3QgbW92ZSB0aGUgYWxsb2NhdGVkIHBvaW50ZXIsIG91ciBhbGxv Y2F0b3IgY2FuIGZyZWUgaXQuCiAJICAgIHBvaW50ZXIgX19kYXRhID0gbnVsbHB0cjsKIAkg ICAgc2l6ZV90eXBlIF9fY2FwYWNpdHk7CiAJICAgIGlmICghX01faXNfbG9jYWwoKSkKIAkg ICAgICB7Ci0JCWlmIChfQWxsb2NfdHJhaXRzOjpfU19hbHdheXNfZXF1YWwoKSkKKwkJaWYg KF9fZXF1YWxfYWxsb2NzKQogCQkgIHsKIAkJICAgIC8vIF9fc3RyIGNhbiByZXVzZSBvdXIg ZXhpc3Rpbmcgc3RvcmFnZS4KIAkJICAgIF9fZGF0YSA9IF9NX2RhdGEoKTsKZGlmZiAtLWdp dCBhL2xpYnN0ZGMrKy12My90ZXN0c3VpdGUvMjFfc3RyaW5ncy9iYXNpY19zdHJpbmcvYWxs b2NhdG9yL2NoYXIvbW92ZV9hc3NpZ24uY2MgYi9saWJzdGRjKystdjMvdGVzdHN1aXRlLzIx X3N0cmluZ3MvYmFzaWNfc3RyaW5nL2FsbG9jYXRvci9jaGFyL21vdmVfYXNzaWduLmNjCmlu ZGV4IGNjNTgzNDhlMTE2Li4yMWUwYjFjYjRmNCAxMDA2NDQKLS0tIGEvbGlic3RkYysrLXYz L3Rlc3RzdWl0ZS8yMV9zdHJpbmdzL2Jhc2ljX3N0cmluZy9hbGxvY2F0b3IvY2hhci9tb3Zl X2Fzc2lnbi5jYworKysgYi9saWJzdGRjKystdjMvdGVzdHN1aXRlLzIxX3N0cmluZ3MvYmFz aWNfc3RyaW5nL2FsbG9jYXRvci9jaGFyL21vdmVfYXNzaWduLmNjCkBAIC0yOCw2ICsyOCw4 IEBAIGNvbnN0IEMgYyA9ICdhJzsKIHVzaW5nIHRyYWl0cyA9IHN0ZDo6Y2hhcl90cmFpdHM8 Qz47CiAKIHVzaW5nIF9fZ251X3Rlc3Q6OnByb3BhZ2F0aW5nX2FsbG9jYXRvcjsKK3VzaW5n IF9fZ251X3Rlc3Q6OnRyYWNrZXJfYWxsb2NhdG9yX2NvdW50ZXI7Cit1c2luZyBfX2dudV90 ZXN0Ojp0cmFja2VyX2FsbG9jYXRvcjsKIAogdm9pZCB0ZXN0MDEoKQogewpAQCAtMTQ2LDEw ICsxNDgsNjAgQEAgdm9pZCB0ZXN0MDMoKQogICBWRVJJRlkoNyA9PSB2OC5nZXRfYWxsb2Nh dG9yKCkuZ2V0X3BlcnNvbmFsaXR5KCkpOwogfQogCit2b2lkIHRlc3QwNCgpCit7CisgIHR5 cGVkZWYgcHJvcGFnYXRpbmdfYWxsb2NhdG9yPEMsIHRydWUsIHRyYWNrZXJfYWxsb2NhdG9y PEM+PiBhbGxvY190eXBlOworICB0eXBlZGVmIHN0ZDo6YmFzaWNfc3RyaW5nPEMsIHRyYWl0 cywgYWxsb2NfdHlwZT4gdGVzdF90eXBlOworCisgIHsKKyAgICB0cmFja2VyX2FsbG9jYXRv cl9jb3VudGVyOjpyZXNldCgpOworICAgIHRlc3RfdHlwZSB2MShhbGxvY190eXBlKDEpKTsK KyAgICB2MSA9ICJhYmNkZWZnaGlqa2xtbm9wcXIxMCI7CisgICAgYXV0byByZWZfYWxsb2Nf Y291bnQgPSB0cmFja2VyX2FsbG9jYXRvcl9jb3VudGVyOjpnZXRfYWxsb2NhdGlvbl9jb3Vu dCgpOworCisgICAgdGVzdF90eXBlIHYyKGFsbG9jX3R5cGUoMikpOworICAgIHYyID0gImFi Y2RlZmdoaWprbG1ub3BxcjIwIjsKKyAgICB2MiA9IHN0ZDo6bW92ZSh2MSk7CisgICAgVkVS SUZZKDEgPT0gdjEuZ2V0X2FsbG9jYXRvcigpLmdldF9wZXJzb25hbGl0eSgpKTsKKyAgICBW RVJJRlkoMSA9PSB2Mi5nZXRfYWxsb2NhdG9yKCkuZ2V0X3BlcnNvbmFsaXR5KCkpOworCisg ICAgVkVSSUZZKCB0cmFja2VyX2FsbG9jYXRvcl9jb3VudGVyOjpnZXRfYWxsb2NhdGlvbl9j b3VudCgpID09IDIgKiByZWZfYWxsb2NfY291bnQgKTsKKyAgICBWRVJJRlkoIHRyYWNrZXJf YWxsb2NhdG9yX2NvdW50ZXI6OmdldF9kZWFsbG9jYXRpb25fY291bnQoKSA9PSByZWZfYWxs b2NfY291bnQgKTsKKworICAgIHYxID0gImFiY2RlZmdoaWprbG1ub3BxcjExIjsKKworICAg IFZFUklGWSggdHJhY2tlcl9hbGxvY2F0b3JfY291bnRlcjo6Z2V0X2FsbG9jYXRpb25fY291 bnQoKSA9PSAzICogcmVmX2FsbG9jX2NvdW50ICk7CisgIH0KKworICB7CisgICAgdHJhY2tl cl9hbGxvY2F0b3JfY291bnRlcjo6cmVzZXQoKTsKKyAgICB0ZXN0X3R5cGUgdjEoYWxsb2Nf dHlwZSgxKSk7CisgICAgdjEgPSAiYWJjZGVmZ2hpamtsbW5vcHFyMTAiOworICAgIGF1dG8g cmVmX2FsbG9jX2NvdW50ID0gdHJhY2tlcl9hbGxvY2F0b3JfY291bnRlcjo6Z2V0X2FsbG9j YXRpb25fY291bnQoKTsKKworICAgIHRlc3RfdHlwZSB2MihhbGxvY190eXBlKDEpKTsKKyAg ICB2MiA9ICJhYmNkZWZnaGlqa2xtbm9wcXIyMCI7CisgICAgdjIgPSBzdGQ6Om1vdmUodjEp OworICAgIFZFUklGWSgxID09IHYxLmdldF9hbGxvY2F0b3IoKS5nZXRfcGVyc29uYWxpdHko KSk7CisgICAgVkVSSUZZKDEgPT0gdjIuZ2V0X2FsbG9jYXRvcigpLmdldF9wZXJzb25hbGl0 eSgpKTsKKworICAgIFZFUklGWSggdHJhY2tlcl9hbGxvY2F0b3JfY291bnRlcjo6Z2V0X2Fs bG9jYXRpb25fY291bnQoKSA9PSAyICogcmVmX2FsbG9jX2NvdW50ICk7CisgICAgVkVSSUZZ KCB0cmFja2VyX2FsbG9jYXRvcl9jb3VudGVyOjpnZXRfZGVhbGxvY2F0aW9uX2NvdW50KCkg PT0gMCApOworCisgICAgdjEgPSAiYWJjZGVmZ2hpamtsbW5vcHFyMTEiOworCisgICAgVkVS SUZZKCB0cmFja2VyX2FsbG9jYXRvcl9jb3VudGVyOjpnZXRfYWxsb2NhdGlvbl9jb3VudCgp ID09IDIgKiByZWZfYWxsb2NfY291bnQgKTsKKyAgfQorCisgIFZFUklGWSggdHJhY2tlcl9h bGxvY2F0b3JfY291bnRlcjo6Z2V0X2FsbG9jYXRpb25fY291bnQoKSA9PQorCSAgdHJhY2tl cl9hbGxvY2F0b3JfY291bnRlcjo6Z2V0X2RlYWxsb2NhdGlvbl9jb3VudCgpICk7Cit9CisK IGludCBtYWluKCkKIHsKICAgdGVzdDAxKCk7CiAgIHRlc3QwMigpOwogICB0ZXN0MDMoKTsK KyAgdGVzdDA0KCk7CiAgIHJldHVybiAwOwogfQo= --------------yeY9jAtKabBRzzsu50hjrAje--