From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 5F72A385B835 for ; Thu, 16 Apr 2020 20:38:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5F72A385B835 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id DC4D31E5F8; Thu, 16 Apr 2020 16:38:36 -0400 (EDT) Subject: Re: [PATCH 28/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412) To: Pedro Alves , gdb-patches@sourceware.org References: <20200414175434.8047-1-palves@redhat.com> <20200414175434.8047-29-palves@redhat.com> From: Simon Marchi Message-ID: <4f6c2c52-2fbb-809f-693c-8ae87d2b8549@simark.ca> Date: Thu, 16 Apr 2020 16:38:36 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, SPF_HELO_PASS, 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:38:38 -0000 On 2020-04-16 4:12 p.m., Pedro Alves via Gdb-patches wrote: > 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. Indeed, I checked "info threads" before posting this, because it's one case I know the deleted thread was referred to. >> 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? The only spot that I found the order mattered was precisely for "info threads". There, I gather the list of threads to display, sort them by per_inf_num, and print them. https://github.com/simark/binutils-gdb/commit/25c67729996dc83912d34a5901016404b3638bc1#diff-5479a5443dca2232f8552dfb5b30ac3dR1104 Simon