[RFC] tweak merge-approval criterion: performance
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
> (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.
More information about the bazaar