From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17571 invoked by alias); 20 Jan 2020 15:05:52 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 17558 invoked by uid 89); 20 Jan 2020 15:05:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=realizing, potential, H*r:2001 X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-2.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (205.139.110.61) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Jan 2020 15:05:42 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1579532740; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=tIKBA7TtYeR7R+vHFLV99WD9Sy0CQ2CVwvL3/jpUt8A=; b=WAjBmLQ4qnpcra1bZS+T8AeLXWe0ipK2xRPH5zwq9k0N09RJwp80iDnapvQpTjXs3HBjE2 DwRA6R0J2iTrfQ1VyDjcbDG/vBBWInAwigwi6k6FG6UlCSXzO1DJh5j8kpX7pIhyRtTIZb 77ZdpVda8SVQz7p5r8VtcYAeI+3Fx1s= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-278-4VjvfgMiOpi2vMtmv_iDoA-1; Mon, 20 Jan 2020 10:05:39 -0500 Received: by mail-wr1-f70.google.com with SMTP id w6so14168873wrm.16 for ; Mon, 20 Jan 2020 07:05:38 -0800 (PST) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:56ee:75ff:fe8d:232b? ([2001:8a0:f913:f700:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id f17sm795680wmc.8.2020.01.20.07.05.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 20 Jan 2020 07:05:37 -0800 (PST) Subject: Re: [PATCH] gdb/linux-fork: simplify one_fork_p To: Christian Biesinger , Simon Marchi References: <20200119161058.864-1-simon.marchi@polymtl.ca> <770454ed-d5b0-4fce-0bf2-a6952f276959@polymtl.ca> Cc: gdb-patches From: Pedro Alves Message-ID: <0e37f7a3-1697-6f90-1eab-f3ff94e47807@redhat.com> Date: Mon, 20 Jan 2020 15:13:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2020-01/txt/msg00606.txt.bz2 On 1/19/20 4:56 PM, Christian Biesinger via gdb-patches wrote: > On Sun, Jan 19, 2020 at 11:53 AM Simon Marchi wrote: >> >> On 2020-01-19 11:41 a.m., Christian Biesinger wrote: >>> On Sun, Jan 19, 2020 at 11:11 AM Simon Marchi wrote: >>>> >>>> Unless I'm missing something, this function is a complicated way of >>>> saying "fork_list.size () == 1". >>> >>> Before C++11, size() wasn't guaranteed to run in constant time, so I >>> assume the code was written to handle that. But GDB uses C++11, so >>> this change seems fine. >>> https://en.cppreference.com/w/cpp/container/list/size >> >> Ahh, good point. Although by the time that change was made, we were already >> using C++11. I don't remember if we had a C++ < 11 phase, but if we did it >> was very short. Yes, we had one. It was short. Everyone hated my unique_ptr emulation so much that we moved quickly to C++11. :-D >> >> Thanks for looking at it, I'll push it now. > > Ah. it's also possible that whoever wrote the code just assumed that > size() would run in linear time, of course. Note, I believe that size() isn't linear when compiled with gcc 4.8, since the new C++11 ABI was only introduced in GCC 5: https://developers.redhat.com/blog/2015/02/05/gcc5-and-the-c11-abi/ But I think it's OK to ignore that. Especially for non-hot code like here. I think it's reasonable to say that if you care about performance, you'll want to compile with a newer compiler. I was the one who wrote it (06974e6c05556e), but I don't remember why I did it that way. Might have been the non-O(1) issue, or it could have been about blindly C++-fying code without realizing the potential simplification. I agree that size () == 1 works just as well, assuming C++11 std::list. Thanks, Pedro Alves