[PATCH] gdb: New frame_cache_cleared observer.

[PATCH] gdb: New frame_cache_cleared observer.

[Andrew Burgess]

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(+)

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)
     {
-- 
2.2.2

Re: [PATCH] gdb: New frame_cache_cleared observer.

[Doug Evans]

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).

Re: [PATCH] gdb: New frame_cache_cleared observer.

[Pedro Alves]

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

Re: [PATCH] gdb: New frame_cache_cleared observer.

[Andrew Burgess]

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!

Re: [PATCH] gdb: New frame_cache_cleared observer.

[Doug Evans]

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).

Re: [PATCH] gdb: New frame_cache_cleared observer.

[Pedro Alves]

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