From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id +DpyMnbZ2mLXsxgAWB0awg (envelope-from ) for ; Fri, 22 Jul 2022 13:08:06 -0400 Received: by simark.ca (Postfix, from userid 112) id CBA9F1E5EA; Fri, 22 Jul 2022 13:08:06 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=glLkeKlz; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from 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 RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 3CF081E222 for ; Fri, 22 Jul 2022 13:08:06 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D5A8C38356B1 for ; Fri, 22 Jul 2022 17:08:05 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D5A8C38356B1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1658509685; bh=f4M96Qk8qUd/dKpqK0uPPfoHMwd2OnOJsgGgphLPB/E=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=glLkeKlzonATEldADqJXrroLGvLDk/CJUj8Nqmjyli9XoCMG1c/F+E7QO5YZGvIHf HUaj7SDfNaEyK9H1/X4jO+Epa3riovdIQqefXwGWgvpt3Gs6UhBuWXDOmoo+YiPc9h sXKH0eDPkvMBORBBP21EkcgexY4DpDjXSrbiHzSk= Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id 7447B38582A9 for ; Fri, 22 Jul 2022 17:07:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7447B38582A9 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id A791C3368E; Fri, 22 Jul 2022 17:07:45 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 8503C13AB3; Fri, 22 Jul 2022 17:07:45 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id xvCbHmHZ2mI4MgAAMHmgww (envelope-from ); Fri, 22 Jul 2022 17:07:45 +0000 Message-ID: <6797f080-f622-e3db-e2e0-47f4d1957f14@suse.de> Date: Fri, 22 Jul 2022 19:07:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH][gdbsupport] Use task size in parallel_for_each Content-Language: en-US To: Pedro Alves , gdb-patches@sourceware.org References: <20220718194219.GA16823@delia.home> <4fc23fcd-c15d-7622-8b51-cc48cd3cba16@palves.net> <75931310-5dcd-059d-9221-6c94dbcd231f@suse.de> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Cc: Tom Tromey Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 7/22/22 02:03, Pedro Alves wrote: > On 2022-07-21 9:23 p.m., Tom de Vries wrote: >> On 7/21/22 19:35, Pedro Alves wrote: >>>> diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h >>>> index bf40f125f0f..3c9269574df 100644 >>>> --- a/gdbsupport/parallel-for.h >>>> +++ b/gdbsupport/parallel-for.h >>>> @@ -134,7 +134,9 @@ typename gdb::detail::par_for_accumulator< >>>>       typename std::result_of::type >>>>     >::result_type >>>>   parallel_for_each (unsigned n, RandomIt first, RandomIt last, >>>> -           RangeFunction callback) >>>> +           RangeFunction callback, >>>> +           std::function *task_size_ptr >>>> +             = (std::function *)nullptr) >>> >>> That use of a std::function pointer looks odd.  AFAICT, TASK_SIZE_PTR is only ever called >>> as a callback by parallel_for_each, for setup, in the calling thread, and isn't stored >>> anywhere, right?  If so, gdb::function_view instead should work, is lightweight, and is >>> nullable, meaning you don't need a pointer. >>> >>> And then, at the caller, just using a lambda instead of a std::function should work too: >>> >>>      auto task_size = [=] (iter_type iter) >>>        { >>>     dwarf2_per_cu_data *per_cu = iter->get (); >>>     return per_cu->length (); >>>        }; >> >> I've tried that (attached) but ran into the usual template error mess, not sure how I could solve that yet. > > I see. The problem is that here: > > @@ -134,7 +135,9 @@ typename gdb::detail::par_for_accumulator< > typename std::result_of::type > >::result_type > parallel_for_each (unsigned n, RandomIt first, RandomIt last, > - RangeFunction callback) > + RangeFunction callback, > + gdb::function_view task_size > + = nullptr) > > parallel_for_each is a template, and the function_view parameter's type depends > on a template parameter (RandomIt), so we can't rely on implicit conversions, such > as when passing a lambda (lambda -> function_view). We need to pass a function_view of > the right type already. That's not special about function_view, it's just how templates > and overload resolution works. > Ack, I usually manage if there's a specific error mesage, but it was too generic in this case. Anyway, I just lack experience writing templated code, something that needs fixing on my side. > So this would fix it: > > diff --git c/gdb/dwarf2/read.c w/gdb/dwarf2/read.c > index 23c3873cba6..06df773f1e0 100644 > --- c/gdb/dwarf2/read.c > +++ w/gdb/dwarf2/read.c > @@ -7067,11 +7067,12 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) > > using iter_type = decltype (per_bfd->all_comp_units.begin ()); > > - auto task_size = [=] (iter_type iter) > + auto task_size_ = [=] (iter_type iter) > { > dwarf2_per_cu_data *per_cu = iter->get (); > return per_cu->length (); > }; > + gdb::function_view task_size = task_size_; > > Though it's annoying to have to write the function_view type. > > Note that this instead, is a bad pattern with function_view (similarly > to other view types, like string_view), as it immediately dangles the lambda > temporary: > > - auto task_size = [=] (iter_type iter) > + gdb::function_view task_size = [=] (iter_type iter) > { > dwarf2_per_cu_data *per_cu = iter->get (); > return per_cu->length (); > }; > > I think the best is to introduce a gdb::make_function_view function, > so that you can do this: > > diff --git c/gdb/dwarf2/read.c w/gdb/dwarf2/read.c > index 23c3873cba6..255b955a54c 100644 > --- c/gdb/dwarf2/read.c > +++ w/gdb/dwarf2/read.c > @@ -7101,7 +7101,7 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) > } > } > return result_type (thread_storage.release (), std::move (errors)); > - }, task_size); > + }, gdb::make_function_view (task_size)); > > /* Only show a given exception a single time. */ > std::unordered_set seen_exceptions; > > I've got that working here. I'll post it tomorrow. > Ack, used in updated patch, submitted in this ( https://sourceware.org/pipermail/gdb-patches/2022-July/191004.html ) series. > Note: the 'task_size' lambda doesn't actually need to capture anything: > > @@ -7067,11 +7067,12 @@ dwarf2_build_psymtabs_hard (dwarf2_per_objfile *per_objfile) > > using iter_type = decltype (per_bfd->all_comp_units.begin ()); > > - auto task_size = [=] (iter_type iter) > + auto task_size = [] (iter_type iter) > { > dwarf2_per_cu_data *per_cu = iter->get (); > return per_cu->length (); > }; > Fixed. Thanks for the review and help. - Tom