Message ID | 1427303468-17834-1-git-send-email-andrew.burgess@embecosm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 112749 invoked by alias); 25 Mar 2015 17:11:22 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 112204 invoked by uid 89); 25 Mar 2015 17:11:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-wg0-f54.google.com Received: from mail-wg0-f54.google.com (HELO mail-wg0-f54.google.com) (74.125.82.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 25 Mar 2015 17:11:20 +0000 Received: by wgdm6 with SMTP id m6so35379712wgd.2 for <gdb-patches@sourceware.org>; Wed, 25 Mar 2015 10:11:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=SP3HcJxOaiqLUqe5HRD9iUEB6YwHD5qtDfpbqXBPGKk=; b=ca3zwus9GyvY6ChKmpQNQHy7Nn3Cu4nKx4hRv9uUmRIfVDs5/tfkS72g/t8ElElkD4 sohbWS1ELV5uNuPycShhFEBm5PE+bJ3RTEcJY5nZKYFoNxfIIQWWjpRM1x07v/IlBceD LBTmRIjqkNz8tGU/F+NhgYIxlHFODxZjNHHvqoigGTV+oPy3RyY5DVc2JBGdU1q+/qjj wlXrodaVcfG4g5lY1eUAINf5wN1pZCe7REEbgyYB6pDoHCovZkqwxodj/A1Coj+TbEXd zPV30BkjmZhYQik4LG6600fWT9ooxZ/vNOX8I+0c+CzfWEEl9WB3Wda8gJUhbkDdeK/G UaLQ== X-Gm-Message-State: ALoCoQkMftjdKKmqyZ//6b72qDWvaxaZv9Fg0AMXM6m14e0GOZNPxE9MbtUXUsBNL9GI/Inc2jnb X-Received: by 10.181.8.103 with SMTP id dj7mr39786220wid.75.1427303477088; Wed, 25 Mar 2015 10:11:17 -0700 (PDT) Received: from localhost (host86-142-42-143.range86-142.btcentralplus.com. [86.142.42.143]) by mx.google.com with ESMTPSA id a6sm5143913wiy.17.2015.03.25.10.11.15 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Mar 2015 10:11:16 -0700 (PDT) From: Andrew Burgess <andrew.burgess@embecosm.com> To: gdb-patches@sourceware.org Cc: Andrew Burgess <andrew.burgess@embecosm.com> Subject: [PATCH] gdb: New frame_cache_cleared observer. Date: Wed, 25 Mar 2015 17:11:08 +0000 Message-Id: <1427303468-17834-1-git-send-email-andrew.burgess@embecosm.com> X-IsSubscribed: yes |
Commit Message
Andrew Burgess
March 25, 2015, 5:11 p.m. UTC
This adds a new observer for the frame cache cleared event. While working on a new gdb port I found that I wanted to cache machine state that was gathered as part of the register read process. The most appropriate time to discard this cached information is when the frame cache is flushed. However, as I don't have an actual use for this observer that I can post upstream (yet) I don't know if this will be acceptable, but given it's a fairly small change I thought I'd try. OK to apply? Thanks, Andrew --- This adds a new frame_cache_cleared observer. gdb/ChangeLog: * frame.c (reinit_frame_cache): Trigger frame_cache_cleared observers. gdb/doc/ChangeLog: * observer.texi (frame_cache_cleared): New event. --- gdb/ChangeLog | 5 +++++ gdb/doc/ChangeLog | 4 ++++ gdb/doc/observer.texi | 4 ++++ gdb/frame.c | 3 +++ 4 files changed, 16 insertions(+)
Comments
On Wed, Mar 25, 2015 at 10:11 AM, Andrew Burgess <andrew.burgess@embecosm.com> wrote: > This adds a new observer for the frame cache cleared event. > > While working on a new gdb port I found that I wanted to cache machine > state that was gathered as part of the register read process. The > most appropriate time to discard this cached information is when the > frame cache is flushed. > > However, as I don't have an actual use for this observer that I can > post upstream (yet) I don't know if this will be acceptable, but given > it's a fairly small change I thought I'd try. > > OK to apply? > > Thanks, > Andrew > > --- > This adds a new frame_cache_cleared observer. > > gdb/ChangeLog: > > * frame.c (reinit_frame_cache): Trigger frame_cache_cleared > observers. Hi. AIUI, our rules for dead code elimination preclude such a patch being applied. I can go either way on this particular patch myself, but those are the rules (as I understand them).
On 03/25/2015 11:18 PM, Doug Evans wrote: > On Wed, Mar 25, 2015 at 10:11 AM, Andrew Burgess > <andrew.burgess@embecosm.com> wrote: >> This adds a new observer for the frame cache cleared event. >> >> While working on a new gdb port I found that I wanted to cache machine >> state that was gathered as part of the register read process. The >> most appropriate time to discard this cached information is when the >> frame cache is flushed. >> >> However, as I don't have an actual use for this observer that I can >> post upstream (yet) I don't know if this will be acceptable, but given >> it's a fairly small change I thought I'd try. ... >> gdb/ChangeLog: >> >> * frame.c (reinit_frame_cache): Trigger frame_cache_cleared >> observers. > > Hi. > > AIUI, our rules for dead code elimination preclude such a patch being applied. > > I can go either way on this particular patch myself, but those are the rules > (as I understand them). > Right. We delete dead code all the time. So it's better to wait until is has a use, because otherwise someone could well end up stumbling on it, noticing it has no uses and decides to send a patch that garbage collects it. Thanks, Pedro Alves
Doug, Pedro, * Pedro Alves <palves@redhat.com> [2015-03-26 09:24:27 +0000]: > On 03/25/2015 11:18 PM, Doug Evans wrote: > > On Wed, Mar 25, 2015 at 10:11 AM, Andrew Burgess > > <andrew.burgess@embecosm.com> wrote: > >> This adds a new observer for the frame cache cleared event. > >> > >> While working on a new gdb port I found that I wanted to cache machine > >> state that was gathered as part of the register read process. The > >> most appropriate time to discard this cached information is when the > >> frame cache is flushed. > >> > >> However, as I don't have an actual use for this observer that I can > >> post upstream (yet) I don't know if this will be acceptable, but given > >> it's a fairly small change I thought I'd try. > Right. We delete dead code all the time. So it's better to wait until > is has a use, because otherwise someone could well end up stumbling on it, > noticing it has no uses and decides to send a patch that garbage > collects it. Thanks for looking at my patch, and I understand why you've rejected it for now. I do have one followup: as far as I can tell the observers register_changed, inferior_call_pre, and inferior_call_post are only used by the python bindings to make the events available in python. As far as I can tell[1] these event bindings are only used within the test suite. ... and a question: If I made frame_cache_cleared a python accessible event, and added a test would this be sufficient to keep the code alive? Thanks for your time, Andrew [1] I could easily be wrong!
On Thu, Mar 26, 2015 at 5:50 AM, Andrew Burgess <andrew.burgess@embecosm.com> wrote: > Doug, Pedro, > > * Pedro Alves <palves@redhat.com> [2015-03-26 09:24:27 +0000]: > >> On 03/25/2015 11:18 PM, Doug Evans wrote: >> > On Wed, Mar 25, 2015 at 10:11 AM, Andrew Burgess >> > <andrew.burgess@embecosm.com> wrote: >> >> This adds a new observer for the frame cache cleared event. >> >> >> >> While working on a new gdb port I found that I wanted to cache machine >> >> state that was gathered as part of the register read process. The >> >> most appropriate time to discard this cached information is when the >> >> frame cache is flushed. >> >> >> >> However, as I don't have an actual use for this observer that I can >> >> post upstream (yet) I don't know if this will be acceptable, but given >> >> it's a fairly small change I thought I'd try. > >> Right. We delete dead code all the time. So it's better to wait until >> is has a use, because otherwise someone could well end up stumbling on it, >> noticing it has no uses and decides to send a patch that garbage >> collects it. > > Thanks for looking at my patch, and I understand why you've rejected > it for now. It's easy enough to prevent people errantly spending cycles submitting a patch to delete such code. I think the larger worry is how long will the code stay in the yet-to-be-used state, and how does one manage things as the quantity of such code grows. IOW, can we manage opening the gates without it turning into a flood? It's just easier to keep the gate shut. > I do have one followup: as far as I can tell the observers > register_changed, inferior_call_pre, and inferior_call_post are only > used by the python bindings to make the events available in python. > As far as I can tell[1] these event bindings are only used within the > test suite. > > ... and a question: If I made frame_cache_cleared a python accessible > event, and added a test would this be sufficient to keep the code > alive? > > Thanks for your time, > Andrew > > [1] I could easily be wrong! Yeah, they're events exported to python and were added because someone had a need for them from python. Original posting is here: https://sourceware.org/ml/gdb-patches/2013-06/msg00889.html Last review cycle begins here: https://sourceware.org/ml/gdb-patches/2014-10/msg00573.html That they're only used by the testsuite is typical of most of python support: It's there for users writing python scripts, gdb itself doesn't use it. If a good use-case can be made for python wanting to know when the frame-cache is cleared then I'd support such a patch. OTOH, I'm wondering if a frame-cache-cleared event is the right one for your use-case. I'm guessing this isn't for frame unwinding, otherwise you could just use the existing mechanism (e.g., frame_unwind.dealloc_cache).
On 03/30/2015 07:51 PM, Doug Evans wrote: > On Thu, Mar 26, 2015 at 5:50 AM, Andrew Burgess > <andrew.burgess@embecosm.com> wrote: >> Doug, Pedro, >> >> * Pedro Alves <palves@redhat.com> [2015-03-26 09:24:27 +0000]: >> >>> On 03/25/2015 11:18 PM, Doug Evans wrote: >>>> On Wed, Mar 25, 2015 at 10:11 AM, Andrew Burgess >>>>> However, as I don't have an actual use for this observer that I can >>>>> post upstream (yet) I don't know if this will be acceptable, but given >>>>> it's a fairly small change I thought I'd try. >> >>> Right. We delete dead code all the time. So it's better to wait until >>> is has a use, because otherwise someone could well end up stumbling on it, >>> noticing it has no uses and decides to send a patch that garbage >>> collects it. >> >> Thanks for looking at my patch, and I understand why you've rejected >> it for now. > > It's easy enough to prevent people errantly spending cycles submitting > a patch to delete such code. If you mean a comment in the code like "don't delete: this will be used by an yet-unsubmitted out of tree port, once it's submitted", I don't agree with that. Should we put a date on the comment? If I read a comment like that saying "2014/09", I'll wonder whether the port will be submitted in the tree soon enough. What about "2013/09"? Or maybe one should bother to look for the right people and ask them if that is maybe dead already? Etc. It's just better to avoid such issues. > OTOH, I'm wondering if a frame-cache-cleared event > is the right one for your use-case. *nod* Without seeing the port it's hard to judge. > I'm guessing this isn't for frame unwinding, > otherwise you could just use the existing mechanism > (e.g., frame_unwind.dealloc_cache). Thanks, Pedro Alves
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index febc377..6d5c757 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,8 @@ +2015-03-25 Andrew Burgess <andrew.burgess@embecosm.com> + + * frame.c (reinit_frame_cache): Trigger frame_cache_cleared + observers. + 2015-03-25 Pedro Alves <palves@redhat.com> * target.h <to_async>: Replace 'callback' and 'context' parameters diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index 30d5a5b..b801f0c 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,7 @@ +2015-03-25 Andrew Burgess <andrew.burgess@embecosm.com> + + * observer.texi (frame_cache_cleared): New event. + 2015-03-24 Pedro Alves <palves@redhat.com> * gdb.texinfo (test_step) <set scheduler-locking step>: No longer diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi index fc3c74a..f33ecb4 100644 --- a/gdb/doc/observer.texi +++ b/gdb/doc/observer.texi @@ -122,6 +122,10 @@ A synchronous command finished. An error was caught while executing a command. @end deftypefun +@deftypefun void frame_cache_cleared (void) +The inferior has had its frame cached cleared. +@end deftypefun + @deftypefun void target_changed (struct target_ops *@var{target}) The target's register contents have changed. @end deftypefun diff --git a/gdb/frame.c b/gdb/frame.c index b3cbf23..94e4f75 100644 --- a/gdb/frame.c +++ b/gdb/frame.c @@ -1690,6 +1690,9 @@ reinit_frame_cache (void) { struct frame_info *fi; + /* Announce cache clearance before releasing memory. */ + observer_notify_frame_cache_cleared (); + /* Tear down all frame caches. */ for (fi = current_frame; fi != NULL; fi = fi->prev) {