Lazy get_frame_id()

Lazy get_frame_id()

[Andrew Cagney]

Hello,

At present a frame's ID is computed when the prev frame is created.  I'd 
like to propose that, instead, for code relying on the new frame 
mechanisms, the computation of a frame's ID be delayed until it is 
needed - an explicit call to get_frame_id().

This will mean that get_prev_frame() is reduced to something like:

	... perform refuse to unwind checks ...
	- this->pc isn't valid? return NULL
	- this->id isn't valid? return NULL
	... create the new frame ...
	prev = allocate()
	prev->pc = frame_pc_unwind (this);
	(prev->type, prev->unwind =
		(frame_type_from_pc (prev->pc),
		 find_unwind_find_by_pc (prev->pc));
	... link it in ...
	this->prev = prev;
	prev->next = this;
	... return it ...
	return prev;


Where:

- frame_pc_unwind(), when applied to the sentinel frame, does not need 
to do any prologue analysis and hence shouldn't be a load on the target.

- frame_type_from_pc(), frame_unwind_find_by_pc() can/should be merged 
since they are both using the same PC based tests.

- the frame ID sanity checks are removed (or only applied when trying to 
do the next chain).


So?

This has two immediatly effects:

- get_current_frame() is cheap

- when a back-trace dies with an invalid frame (due to frame ID), it is 
possible to examine that invalid frame (previously it wasn't possible).

It opens up the possibility of, in WFI (infrun.c, et.al.), replacing the 
calls:
- PC_IN_SIGTRAMP()
- PC_IN_CALL_DUMMY()
with the equivalent: get_frame_type (get_current_frame ()) == 
SIGTRAMP_FRAME et.al.  In fact, since the computation would be done 
once, this would be less load on the target.


What's the fine print?

It will have the effect of making it very difficult to predict which of 
this_id() or prev_register() is called first.  The call sequence can 
always be forced (if that makes peoples life easier).

``for code relying on the new frame mechanisms''.  The WFI change, for 
old architectures, could potentially make them run slower (for old 
architectures get_current_frame() forces a prologue analysis).  I'm 
going to go out on a limb and argue that this is OK.  It doesn't break 
the old architectures (just degrades them a little :-).  On the other 
hand, it makes it possible to ratinalize WFI a little.


Anyway, ignoring WFI, any comments on the principle behind this change?


Thoughts?
Andrew

Re: Lazy get_frame_id()

[Daniel Jacobowitz]

On Sun, Mar 16, 2003 at 06:27:19PM -0500, Andrew Cagney wrote:
> Hello,
> 
> At present a frame's ID is computed when the prev frame is created.  I'd 
> like to propose that, instead, for code relying on the new frame 
> mechanisms, the computation of a frame's ID be delayed until it is 
> needed - an explicit call to get_frame_id().
> 
> This will mean that get_prev_frame() is reduced to something like:
> 
> 	... perform refuse to unwind checks ...
> 	- this->pc isn't valid? return NULL
> 	- this->id isn't valid? return NULL
> 	... create the new frame ...
> 	prev = allocate()
> 	prev->pc = frame_pc_unwind (this);
> 	(prev->type, prev->unwind =
> 		(frame_type_from_pc (prev->pc),
> 		 find_unwind_find_by_pc (prev->pc));
> 	... link it in ...
> 	this->prev = prev;
> 	prev->next = this;
> 	... return it ...
> 	return prev;
> 
> 
> Where:
> 
> - frame_pc_unwind(), when applied to the sentinel frame, does not need 
> to do any prologue analysis and hence shouldn't be a load on the target.
> 
> - frame_type_from_pc(), frame_unwind_find_by_pc() can/should be merged 
> since they are both using the same PC based tests.
> 
> - the frame ID sanity checks are removed (or only applied when trying to 
> do the next chain).

This I really like.  It seems like the right time...

> This has two immediatly effects:
> 
> - get_current_frame() is cheap
> 
> - when a back-trace dies with an invalid frame (due to frame ID), it is 
> possible to examine that invalid frame (previously it wasn't possible).

This is a definite improvement.  You mean if (for instance) there is a
call to error() trying to build the next frame?  I've run into this in
practice.

> It opens up the possibility of, in WFI (infrun.c, et.al.), replacing the 
> calls:
> - PC_IN_SIGTRAMP()
> - PC_IN_CALL_DUMMY()
> with the equivalent: get_frame_type (get_current_frame ()) == 
> SIGTRAMP_FRAME et.al.  In fact, since the computation would be done 
> once, this would be less load on the target.
> 
> 
> What's the fine print?
> 
> It will have the effect of making it very difficult to predict which of 
> this_id() or prev_register() is called first.  The call sequence can 
> always be forced (if that makes peoples life easier).

IMVHO, we should probably force the frame ID before general register
unwinding.

> ``for code relying on the new frame mechanisms''.  The WFI change, for 
> old architectures, could potentially make them run slower (for old 
> architectures get_current_frame() forces a prologue analysis).  I'm 
> going to go out on a limb and argue that this is OK.  It doesn't break 
> the old architectures (just degrades them a little :-).  On the other 
> hand, it makes it possible to ratinalize WFI a little.

And I agree here too.  Mark's making great strides on converting i386
already.  I don't have time this month to follow the frame stuff as it
evolves, so I've been mostly ignoring it, but I'm willing to try my
hand at converting the other architectures I need once it seems to have
stabilized.

If your port wants performance benefits it has to move forwards!

> Anyway, ignoring WFI, any comments on the principle behind this change?

I like it.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Lazy get_frame_id()

[Andrew Cagney]

> It opens up the possibility of, in WFI (infrun.c, et.al.), replacing the calls:
> - PC_IN_SIGTRAMP()
> - PC_IN_CALL_DUMMY()
> with the equivalent: get_frame_type (get_current_frame ()) == SIGTRAMP_FRAME et.al.  In fact, since the computation would be done once, this would be less load on the target.

So, I've got some raw GCOV data vis:

	new	lazy	main	func

         4092	4587	4587	gdbarch_pc_in_sigtramp
	9456	9246	9385	gdbarch_deprecated_pc_in_call_dummy
	3047	2837	2837	create_sentinel_frame
	3739	3739	3882	d10v_frame_this_id
	4296	4296	4331	d10v_frame_prev_register
	35754	35754	36281	gdbsim_xfer_inferior_memory
	24434	24434	24581	gdbsim_fetch_register

main=mainline
lazy=lazy id evaluation
new=lazy id evaluation + infrun using get_frame_type()

but am wondering how to interpret it.  Here's an attempt:

The create sentinel frame's went up slightly as it is creating more 
frames.  This would also cause the slight rise in pc_in_call_dummy calls 
(since that is always checked first).  The minor this_id, prev_register, 
and the gdbsim_XXX call reduction can be attributed entirely to the lazy 
id evaluation (the d10v sigtramp function doesn't do anything to the 
target).

The important one is pc-in-sigtramp and that was reduced by 10%.

So I guess this data indicates that all is ok ...  Any suggestions for 
what else to look at?  Mainline with infrun using get_prev_frame()?

Andrew

PS: There is also calls like:
PC_IN_SIGTRAMP (prev_pc, prev_func_name)
but I left them them be.