From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com [IPv6:2a00:1450:4864:20::52e]) by sourceware.org (Postfix) with ESMTPS id 8BDAF385841B; Mon, 20 Nov 2023 16:46:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8BDAF385841B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8BDAF385841B Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::52e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700498811; cv=none; b=daPTgJGqxLT5IXodfEjsabRQZa5RU9jnNRzMuvaGRn8z/TRYuAl5JD/ufZ3ede6+SCOlE8SriaeETudBB4vlcq60wPsuN2Ph7ZzJugm1KViw5cyXKPidWBOYustQLs7ZU0a+QNpIIbH/jJNygkCQUyvwx9euDs+QE3ZLjQ6p1FQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1700498811; c=relaxed/simple; bh=BfjY2yv2kKyIzRDQxEQfTJ0Yysvw9eG4N3VZGAOP0Oo=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=na2yM/An+nxZrxUY1wh65jn3MM1Jp+ldn5Ozj58xLm0wZSyoCc65be4uo2GOGYVXzWw87X4OT1ha6wsHeiqdzmEVhwmIevJKPMjHTmqrTiKoR6wrG0V4XfqxQ+DbHtq4EKbu5K3IyMxtyanI3YuZCTgHg2V31qd+cfs/wVb94Mc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ed1-x52e.google.com with SMTP id 4fb4d7f45d1cf-548d1f8b388so1321235a12.0; Mon, 20 Nov 2023 08:46:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1700498808; x=1701103608; darn=gcc.gnu.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=gW133snQ3YseGpG2gS4JPGkCUz36JDoyj3mVfKg06sM=; b=R+WJR+igpJQho58b5CghrFF4R6E8vo3OVKtb1khNjWYw/cYdS0FTclQVQ3dJt7EPQ6 oriG7ElYEdlbV4QLEhLV7Klg1ygfc9rRPwtN1jJjhFGZ5BPcO/HQpaEuvGCvv56iyH4p LBDwF+syaKSw7HSClLVyfbIe51Y7Hwbww3rO6o5DaqvVgEJgPz/1Kj/ad+k1l25Ca4H2 ZnbCoQwTq2TngIvYR3MKxGPAlD0nKtdUFMVWmxaqqw4YYsDWBBKq5Vr3CRzbWVMB2Vby x5tjWZGt7yKf//9fgT8IZkZecflApSoRWuohNj5vAS1075K+1wG/Tn/fvhy6MUZE0vQO Eu5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700498808; x=1701103608; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=gW133snQ3YseGpG2gS4JPGkCUz36JDoyj3mVfKg06sM=; b=h3mcCavKGDFwI5rqj+Z0GQa2bJoxiZ3hsjp1DHq/0zvpGDaxmu8prMVYGmy1T/kivG BF0h+gbo7gJfTmnsoUD9sZ4x96bZaNIT3ICfz+yOO6VfwFLmrqkDOi8HpDht0Iy7Q+n5 hJF8UBhTBj1ENiUk4UcQucyTSvTsZDp7qBWtkz+/PLrRneEW5Gnyz8gt6c9iUVQcyH71 xz6Ed04GXzIu1ahKvcwuawsXDJQge6NRqT2alKsNrrCpIuXXWhNzq/Qbx11+pr2fhKR5 vSKnaS1dfSZlr154Nkkr5CW5y8I7Ieb74PvxT8B32e1K22zBetm6M576cFfpQScOMKVV +4GA== X-Gm-Message-State: AOJu0YzQv4+9jsGdH+SczUq6vF2CKJvE8FIZeKuE1JrAsFAxEcNYBtI4 XSiq8jaRyDAmopAcj6e6uBzo5ghYiQ91XxPf3CM= X-Google-Smtp-Source: AGHT+IEUDwfYcu+6BFTB+oEID1+AzvLNumvZFTVRkfhDTcCKvunx1sgDAcOkAn2hCoxPZ3TzvWXOznyHC2fYBqHD0GU= X-Received: by 2002:a17:906:210:b0:9e4:b664:baa8 with SMTP id 16-20020a170906021000b009e4b664baa8mr5570723ejd.7.1700498807344; Mon, 20 Nov 2023 08:46:47 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Jonathan Wakely Date: Mon, 20 Nov 2023 16:46:34 +0000 Message-ID: Subject: Re: libstdc++: Speed up push_back To: Jan Hubicka Cc: Jonathan Wakely , libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-0.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: On Mon, 20 Nov 2023 at 15:44, Jan Hubicka wrote: > > > > + // RAII type to destroy initialized elements. > > > > There's only one initialized element, not "elements". > > > > > + struct _Guard_elts > > > + { > > > + pointer _M_first, _M_last; // Elements to destroy > > > > We only need to store one pointer here, call it _M_p. > > > > > + _Tp_alloc_type& _M_alloc; > > > + > > > + _GLIBCXX20_CONSTEXPR > > > + _Guard_elts(pointer __elt, _Tp_alloc_type& __a) > > > + : _M_first(__elt), _M_last(__elt + 1), _M_alloc(__a) > > > + { } > > > + > > > + _GLIBCXX20_CONSTEXPR > > > + ~_Guard_elts() > > > + { std::_Destroy(_M_first, _M_last, _M_alloc); } > > > > This should be either: > > > > std::_Destroy(_M_p, _M_p+1, _M_alloc); > > > > or avoid the loop that happens in that _Destroy function: > > > > _Alloc_traits::destroy(_M_alloc, _M_p); > > > > > + > > > + private: > > > + _Guard_elts(const _Guard_elts&); > > > + }; > > > + > > > + // Guard the new element so it will be destroyed if anything throws. > > > + _Guard_elts __guard_elts(__new_start + __elems, _M_impl); > > > + > > > + __new_finish = std::__uninitialized_move_if_noexcept_a( > > > + __old_start, __old_finish, > > > + __new_start, _M_get_Tp_allocator()); > > > + > > > + ++__new_finish; > > > + // Guard everything before the new element too. > > > + __guard_elts._M_first = __new_start; > > > > This seems redundant, we're not doing any more insertions now, and so > > this store is dead. > > I removed this one. > > > > > + > > > + // New storage has been fully initialized, destroy the old elements. > > > + __guard_elts._M_first = __old_start; > > > + __guard_elts._M_last = __old_finish; > > However here I think I need __guard_elts supporting destruction of many > elements in case the vector has moved to new location.... Ah yes. > So I do not quite see how to simplify the code as suggested above > except that the constructor can be simplified to not require first and > last argument since we always initialize it for 1 destruction but later > we may update it. We could just destroy the old ones manually at the end of this block, it doesn't need to be done by the guard, e.g. update the guard to destroy the first one, then loop to destroy the others: __guard_elt._M_p = __old_start++; std::_Destroy(__old_start, __old_finish, this->_M_impl); But this would only improve codegen in the exceptional case when something throws, and the guard destructor destroys a single element. For the common case where we reach the end of the block, we're always going to loop. So I agree with leaving __guard_elts in charge of two pointers, and looping in its dtor. So OK for trunk, with the dead store removed. Thanks!