[m-rev.] For post-commit review: Coverage profiler no-longer instruments deep profiler instrumentation goals.

[Paul Bone]

On Tue, Sep 16, 2008 at 06:56:19PM +1000, Zoltan Somogyi wrote:
> On 14-Sep-2008, Paul Bone <pbone at csse.unimelb.edu.au> wrote:
> > +        % XXX: This computation seems broken, it's been disabled so I can
> > +        % relyably implement coverage profiling.  Test it on
> > +        % erlang_rtti_implementation.deconstruct_2/9-2 whose switch's type has
> > +        % 25 constructors yet this computes 27.  Are constructors different to
> > +        % functors?
> 
> I visually checked erlang_rtti_implementation.deconstruct_2, and the switch
> variable in it DOES have 27 constructors; the switch covers them all.
> The switch has 25 arms, because the last arm is shared by three function
> symbols.
> 
> What problem did you think you were fixing by disabling this code?

I mis-understood the second value in the step_switch/2 constructor, and
beleived it was the number of arms in the switch (which is unnecessary)
Now I understand it's the number of constructors in the switched-on
type.

The problem is that the goal paths included with the call sites and
coverage points contain this number, which cannot be calculated when the
program is being traversed by the deep profiling tools (for example, the
coverage propagation code), since they don't have access to type
information.

So by removing this calculation and omitting the count of the type's
constructors I can avoid this problem.  There are probably better ways
to avoid it though.  I can either update my comment in this code, or
avoid it in another way by modifing how the coverage propagation code
works.

I'll cover anything else in this review or in your changes seperatly.

Thanks.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20080916/0c652223/attachment.sig>