[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