[RFC] tweak merge-approval criterion: performance

Ian Clatworthy ian.clatworthy at internode.on.net
Tue Sep 4 01:21:43 BST 2007


Robert Collins wrote:

> So I propose that we split performance out into a fourth gate of its own
> - 'does not reduce performance of performance sensitive commands', and
> start profiling any patch that looks likely to impact
> commit/status/diff/log/push/pull/branch/merge on trees likely to show
> scaling or constant time problems.

I agree with the gate.

I'm not sure profiling every potential change by hand is practical. If
it's automated PQM-like, that's different. Until then, I think some
focussed performance regression testing during the freeze period - which
I've been doing for some months BTW - together with keeping up good
reviews is acceptable.

As we all know, lots and lots of changes - particularly anything low
level - can dramatically improve some areas while making others worse.
That's why I favour explicitly testing system performance during feature
freeze regardless.

> (The merge of the pre-commit hook without profiling is what has
> triggered this suggestion)

Thanks for being picky about this. Commit speed matters a lot to most
users. FWIW, there was a short discussion on pre-commit hook performance
on either IRC or the mailing list - enough for me to be comfortable merging.

Just profiled this now as well. Committing 6 new files to a Mozilla tree
using bzr.dev (2790) when no pre-commit hooks are defined (the normal
case) takes 46,869 ticks. 0 ticks were taken by the pre-commit hook
processing. That was expected - it's one extra fn call per commit that
runs an if test and returns.

OTOH, the profiling when a pre-commit hook is defined might show
something really bad of course. I'll try that today. Even so, adding
pre-commit hooks as a feature was an incremental improvement and
therefore it was acceptable to merge IMO.

Ian C.



More information about the bazaar mailing list