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 AB4743858C54 for ; Tue, 4 Apr 2023 15:06:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AB4743858C54 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.170] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 3742A1E110; Tue, 4 Apr 2023 11:06:21 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1680620782; bh=ExUlCxmNYP1F+g9JazdJDVU4PzP/F75ITpU5GPYNwwQ=; h=Date:Subject:To:References:From:In-Reply-To:From; b=uz8jyClCYxJeI8duzGo/+TVZyEr9t2CGDsIvpy4dH2EsdAbNq6nA502hJdatFtXTq yKEEpGtvJllr4iKwkTTFLzTupgrJowErUUZblVaujfrfL8uVgvd3IgJ42GcZTIrsIA m8W51QlBkh9SgkH4UbquHLoi+f9P3eKKavKTdP5Q= Message-ID: Date: Tue, 4 Apr 2023 11:06:21 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [PATCH v8 09/10] btrace, python: Enable ptwrite filter registration. Content-Language: fr To: "Willgerodt, Felix" , "gdb-patches@sourceware.org" References: <20230321154626.448816-1-felix.willgerodt@intel.com> <20230321154626.448816-10-felix.willgerodt@intel.com> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: > I agree on the doc part. It would indeed be nice to write this down. > > Though, I think you misunderstood some things (or maybe I did). > Gdb.current_recording().clear(), as added in patch 5, does clear on a > per-thread basis. Not on a per inferior basis as you write. > But maybe you meant _clear_traces() here? This is supposed to be > an internal function, as the leading underscore suggests. It indeed > does clear all recordings for all threads. As that is needed for ptwrite, > which is the only intended use case of this internal function. > > So I don't quite see what would be unexpected. My misunderstanding comes from where you said: > Not sure if it is worth exiting early per inferior. I couldn't find > a scenario where you can selectively record (or not record) > one thread of a multi-threaded inferior. By "exiting early", I thought you meant that it would be enough to clear the data for one thread in each inferior. >> I understand that you want to have one copy of the filter per thread. >> >> But doing a copy seems like a strange instantiation strategy. The >> "global" filter isn't ever used, right? It only serves to make copies? >> I think a more common pattern would be for the user to register a >> factory function (really, ny callable that fits the bill) which would be >> called whenever we need to create an instance. > > Mhm, I think that would make this a bit too complex for users. > I don't think they want to care about providing a generator as well > that returns a callable. I don't see any new feature that > is being unlocked with that, just more tasks for the user. I don't think it adds significant complexity, this is pretty typical Python. It is similar to what we have for the pretty printers. You register a function that gets called to instantiate the right pretty printer. At the simplest, it would be: class MyFilter: ... def make_one_filter(): return MyFilter() register_filter(make_one_filter) or even: class MyFilter: ... register_filter(lambda: MyFilter()) An advantage is that it makes it easy to return a different filter per thread, if needed. Since filters are on a per-thread basis, I think it would even make sense to pass the thread to the factory function, in case the user wants to select a filter based on that. Not that the fact that filters are callable themselves is just a design choice you made, you could very well decide to call something else than __call__ on those objects. If you want to make things more explicit, you can make it so filters have to implement a "get_aux_data" method, instead of __call__. The advantage of using __call__ is just that it allows freely passing functions as well as objects implementing __call__, so that gives the user a bit more flexibility. Simon