[m-rev.] Updated diff for deep profiling.

Zoltan Somogyi zs at cs.mu.OZ.AU
Tue May 29 17:38:34 AEST 2001


On 29-May-2001, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> Oh, so you did a cvs update between producing the old diff and
> producing the new diff?  And you didn't create a relative diff
> before doing the cvs update?

I did, but it was obsoleted by the merges I did to resolve the conflicts
resulting from the cvs update. If I didn't do the CVS update, then the changes
I first made on the deep profiling branch that I later separated out to commit
in pieces on the trunk would have confused the issue. And as a practical
matter, I needed to perform CVS updates to gain access to new capabilities,
such as mode-specific clauses to replace model_non pragma C code.

> And you didn't backup the workspace
> before/after doing the cvs update?

Well, the workspace is about 1.5 Gb in size, since it is compiled with several
kinds of debugging. Backing it up takes a significant fraction of an hour.
Sometimes this is too long to wait when you want to work on something right
now. There is also a limit to how many copies of workspaces my laptop can hold.

> 1.  That presumes there is only one reviewer.  But there may be more than one.
>     Indeed, since code review is an extremely effective way of reducing
>     later maintenance effort, we should be willing to expend significant
>     effort to encourage multiple reviewers.

That is a nice ideal, but unrealistic; we all have our own work to do.
Very few changes get second reviews, and I haven't noticed any correlation
between effort invested in reviewability and the probability of multiple
reviews.

>     If, on the other hand, you go ahead and hack away for a few months,
>     without thinking about what you can do to ensure that the changes
>     will be easy to review, and produce a single huge diff without
>     considering whether it could or should be posted in separate parts to
>     make it easier to review,

Deep profiling is a complex system; it is inevitable that the diff is large.
The parts I could separate out, such as avoiding model_non pragma C code and
optimizing calls and pragmas with determinism failure, I had reviewed and
committed separately.

>     and don't bother to backup your workspace
>     at appropriate points (like when posting a diff, and [if you've
>     already posted a diff] before/after doing a cvs update), making it
>     difficult to produce a relative diff, then yes, it will be harder.
>     But I don't think we should encourage that -- do you?

No, we shouldn't. I intended to use interdiff to generate the relative diff,
but since it doesn't have a manual, I did not know about its limitations until
too late. Is this any worse a mistake than committing changes that break the
build because you did not do a bootcheck before committing, a mistake you have
made several times?

Assigning blame may feel good, but it is not a productive exercise. I told you
how you can use the diff I posted: check your review points against it. Will
you or won't you?

Zoltan.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list