From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by sourceware.org (Postfix) with ESMTP id D4A70385DC00 for ; Thu, 16 Apr 2020 20:12:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D4A70385DC00 Received: from mail-ej1-f70.google.com (mail-ej1-f70.google.com [209.85.218.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-353-nVQixFeeMtmAUAb67S3IfA-1; Thu, 16 Apr 2020 16:12:49 -0400 X-MC-Unique: nVQixFeeMtmAUAb67S3IfA-1 Received: by mail-ej1-f70.google.com with SMTP id b23so1441441ejv.5 for ; Thu, 16 Apr 2020 13:12:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Pl/0jow9CFXNzBBfqGPnREAtlGsfQpIQy54xuhfmz9g=; b=nfh1K49bv07CYC8vulUrtL5Ve/pelKFi1XQANRokjLiJM3N0y5uunt10c+CAkII5LU OX0qcqQP6h6kcRB57kj+VMXtCowbuIscDwGsME1PMbFZtYbRgnNBaefXm3lLq61Kujbn Cl3jVVUra+cUx+esKkSei8KoqRpFXEhYalNzZWvM0VwJPwxe9ar2OJe0i9529trJfXn2 cNgKoqOHo7s9gN80Dl2wtp6hKzImsBX4fy1TYH+bFBddRQ4qjJ3h2tZIXd9k51syni/J wBvSRxHij1SF9cB7nyveUv5Q/nzo7mB8lpNg9EHea/7hrZz340NxAi2o5f9XunViyWWg +BWg== X-Gm-Message-State: AGi0PuYesoGKYaZYOtQzdN65hOGvIIOyD1p3j//JXo0uHu0eyFtZWIpc QgxPFK/wwwP9ULt1W4DrwKhbwm4ptUDPgnoNmHotjvQcEC9Xfhzic32f1ffjVgtwo1og6iJAy/P Rljgf4CfEc2dXVYWE5e9j4Q== X-Received: by 2002:a50:d596:: with SMTP id v22mr3057088edi.91.1587067966695; Thu, 16 Apr 2020 13:12:46 -0700 (PDT) X-Google-Smtp-Source: APiQypK7NEx+nfMvHTW8Jv9fBBCm5a6nUwA4s4TkshxgYR2D6klR7fAO2vMXyp5rzhPU3/FA6RXQjQ== X-Received: by 2002:a50:d596:: with SMTP id v22mr3057043edi.91.1587067965998; Thu, 16 Apr 2020 13:12:45 -0700 (PDT) Received: from ?IPv6:2001:8a0:f909:7b00:56ee:75ff:fe8d:232b? ([2001:8a0:f909:7b00:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id n17sm1874867eja.9.2020.04.16.13.12.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Apr 2020 13:12:45 -0700 (PDT) Subject: Re: [PATCH 28/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) To: Simon Marchi , gdb-patches@sourceware.org References: <20200414175434.8047-1-palves@redhat.com> <20200414175434.8047-29-palves@redhat.com> From: Pedro Alves Message-ID: Date: Thu, 16 Apr 2020 21:12:43 +0100 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: Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Apr 2020 20:12:54 -0000 On 4/16/20 8:39 PM, Simon Marchi wrote: > I checked the rest of the series, I didn't find any obvious problem. Thanks. > > I have a patch [1] that I might consider sending upstream some day, which replaces the > inferior thread lists with maps, using the ptid as the key. This was made for targets > that have a lot of threads (like, thousands), where looking up a thread object from a > ptid would take a bit of time, and that would add up. If there can be two threads in > the list with the same ptid, that won't work with a map using the ptid as the key. > > So I was wondering, when we want to delete a thread but its refcount is not 0, does it > need to stay in the thread list? What if we remove it from the list, and whatever > decrements its refcount to 0 deletes it? Do you think that could work? I think it could. That's something that I considered, as something that is made possible with this patchset, because, before we needed it to stay in the list so that the lookup inside inferior_thread(), but after the series the lookup is gone. We'd also need to look out for places that want to walk all threads, instead of just non-exited ones, but on a quick look, I didn't find any left that couldn't be converted to walk to non-exited threads only. Another thing that makes it possible is that "info threads" doesn't show the exited threads in the list. Even if we wanted to show the current thread in the list if it's exited (which is something I considered), we could still easily do that, though. Currently, we just show the "The current thread has terminated. See `help thread'" message at the end of the thread list. > > It's possible, however, that with your series, the number of times we look up a thread > from its ptid is reduced enough that it makes my patch not really useful. > > Simon > > [1] https://github.com/simark/binutils-gdb/commits/inferior-thread-map How are you making sure that iterating the threads walks them in creation/insertion order instead of ptid_t order? Thanks, Pedro Alves