From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id ewW4MbJ6TGhC4gwAWB0awg (envelope-from ) for ; Fri, 13 Jun 2025 15:23:30 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=polymtl.ca header.i=@polymtl.ca header.a=rsa-sha256 header.s=default header.b=gCOal8Kc; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id B2B591E102; Fri, 13 Jun 2025 15:23:30 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-9.1 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED,RCVD_IN_VALIDITY_RPBL, RCVD_IN_VALIDITY_SAFE autolearn=unavailable autolearn_force=no version=4.0.1 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id EECF01E0C2 for ; Fri, 13 Jun 2025 15:23:29 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7BD0A380ED05 for ; Fri, 13 Jun 2025 19:23:29 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7BD0A380ED05 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=polymtl.ca header.i=@polymtl.ca header.a=rsa-sha256 header.s=default header.b=gCOal8Kc Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 5210D380ED04 for ; Fri, 13 Jun 2025 19:22:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5210D380ED04 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5210D380ED04 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=132.207.4.11 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1749842541; cv=none; b=NxraUVdWvMFpXKgVGoX7c89o+wwm8q9vCQYTWWhGL5N86sBIlmAydhn8IqJ8GYDwWd1BKVRFFnt/ZyQcpRDDuHZhNvw3TPGWO/l3sOnyPhk0NykP/nDT8UWbN5QsBze6M1z3kB4qxkQKLa0EzwSiN+faNPtnSuqJF6B/a4T9Yp8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1749842541; c=relaxed/simple; bh=3bjaWsuR5Vl3nVLnS1yZvzs9IVy7bhGF3OP6RY4g3i0=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=tDYIx3VbvuvNQchDcvA2gvkz8TlL6JpttMiDW/rqmE/48F/4V2xHmAkQCWXxGiov6pDOdwFfoYEKZnF8u5kM+F+MsAyOhMi0Qf5Vm7iXBcV0VlXvOC223NTRsx0j7k0wvYrxrpW5YKRMwQ4brfvwDXT0rYqUjGTqRhGE+g3T8a0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5210D380ED04 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 55DJMF9b046468 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 13 Jun 2025 15:22:20 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 55DJMF9b046468 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1749842540; bh=KnaGhfdDvzTb5P9h2x5BU6xgvttfaVeB6XM56WO1C9o=; h=Date:Subject:To:Cc:From:In-Reply-To:From; b=gCOal8KceiDtkHl93A8IGHKGl6l0XFtYKl3jRbjj/KGz0/suWB+6XPt/gStoQfBBs KR4MVjvezqiPnOpCPgnMa0qsS1LoL/QUo40ClbIA6clOVM1JuIu09WV6/psVaMm2nf n0zqc+a9bDq5hPBdklI5CPbfbyWO54hqlhE/MDII= Received: by simark.ca (Postfix, from userid 112) id D5F021E102; Fri, 13 Jun 2025 15:22:15 -0400 (EDT) Received: from [172.16.0.192] (192-222-132-26.qc.cable.ebox.net [192.222.132.26]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 0F19E1E0C2; Fri, 13 Jun 2025 15:22:13 -0400 (EDT) Message-ID: <46f028fd-14f8-49a5-afc1-d6661d6f84ac@polymtl.ca> Date: Fri, 13 Jun 2025 15:22:13 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/6] gdbsupport: use dynamic partitioning in gdb::parallel_for_each To: Tom Tromey , Simon Marchi Cc: gdb-patches@sourceware.org References: <20250505201548.184917-1-simon.marchi@efficios.com> <20250505201548.184917-3-simon.marchi@efficios.com> <87tt4jjy13.fsf@tromey.com> Content-Language: fr From: Simon Marchi In-Reply-To: <87tt4jjy13.fsf@tromey.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 13 Jun 2025 19:22:16 +0000 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 6/13/25 2:29 PM, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi writes: > > Simon> - I want to still be able to use scoped_time_it to measure the time > Simon> that each worker thread spent working on the task. I find it really > Simon> handy when measuring the performance impact of changes. > > Simon> Unfortunately, the current interface of gdb::parallel_for_each, which > Simon> receives a simple callback, is not well-suited for that, once I > Simon> introduce the dynamic partitioning. The callback would get called > Simon> once for each work item batch (multiple time for each worker thread), > Simon> so it's not possible to maintain a per-worker thread object for the > Simon> duration of the parallel for. > > Simon> To allow this, I changed gdb::parallel_for_each to receive a worker > Simon> type as a template parameter. Each worker thread creates one local > Simon> instance of that type, and calls operator() on it for each work item > Simon> batch. By having a scoped_time_it object as a field of that worker, > Simon> we can get the timings per worker thread. > > Simon> The drawbacks of this approach is that we must now define the > Simon> parallel task in a separate class and manually capture any context we > Simon> need as fields of that class. > > This seems like a real step backward to me. It's basically replicating > lambdas by hand. > > Why can't parallel_for_each just do this itself? It could take an > argument naming the task to pass to the scoped_time_it constructor. The > 'task' lambda could instantiate the timer. This would require scoped_time_it to be in gdbsupport. For the moment, scoped_time_it is defined in gdb, because it was easier to do so at the time, but it could be moved to gdbsupport. In the patch that makes the DWARF reader use gdb::parallel_for_each_async, we keep more per-worker state, the `cooked_index_worker_result` where each worker threads puts what it computes. We would need a way to make that work with the lambda. One way to do this would be to pass to the lambda the index (0 to N-1) of the worker thread, and the lambda could use it to index into the vector of cooked_index_worker_result, which would have been prepared by the caller of gdb::parallel_for_each_async. The last detail would be: how do to some one-time work at the end of a parallel for-each worker's life. For example, in the DWARF indexer, each worker does: - call bfd_thread_cleanup - move the complaints to the thread storage - move the thread storage to the results vector How could this work, given that the lambda is invoked many times for each worker? We could make gdb::parallel_for_each accept a second lambda, "finalize", which would be called at the end of a worker's life. Another idea I had (but didn't try) was to make lambdas receive a "magic" range object, like athis: [&] (gdb::dynamic_range range) { for (dwarf2_per_cu *cu : range) process_unit (cu); } "range" would get work items from the work queue in batches, but yield one at a time. It would reach its end when the work queue is empty. It will be a bit of work to implement, but it would have the advantage that any per-worker state could be right there in the lambda, like we have today. Any thoughts? > Simon> + /* The next item to hand out. */ > Simon> + std::atomic next = first; > > This is maybe a bit worrying since IIUC std::atomic only works for > trivially copyable types. But this doesn't have to be a general purpose > facility, it only has to work for gdb's needs. Yes, I hit this when trying to do: gdb::parallel_for_each (vector.begin (), vector.end (), ...); By default, a vector iterator is small and trivial. But when building with -D_GLIBCXX_DEBUG, then it's no longer trivially copyable. That's why I used the less ideal: gdb::parallel_for_each (vector.data (), vector.data () + vector.size (), ...); If you have a better idea to make this work, let me know. Simon