On Wednesday 06 April 2005 08:14 am, Tres Seaver wrote: > Florent Guillaume wrote: > > Dieter Maurer <[EMAIL PROTECTED]> wrote: > >>In our regular profiles, "listFilteredActionsFor" belongs to > >>the top consumers of CPU time. > >> > >>Recently, I found the main culprit (in CMF 1.4): > >> > >> It is the completely unnecessary: > >> > >> if not action in catlist: > >> > >>In our case, "listFilteredActionsFor" spends about 70 percent > >>of its complete time in the checking of "action in catlist". > >> > >>How in hell should the same action be defined more than once > >>such that we need to prevent such a case by an explicit check -- > >>especially by such an expensive one? > >> > >>A comment before the line indicates that the author intended > >>to check by identity. But, of course, "action in catlist" > >>does *NOT* check by identity but by equality. > >> > >> > >>I propose to remove the check altogether... > > > > +1. > > > > The three lines above could be reduced to one using .setdefault() too. > > We could also use a 'seen' dictionary: > > key = (category, action.id) > if key not in seen: > seen[key] = 1 > ....
That would be changing behavior, and the result may be undesirable to some (as yuppie indicates above). I think it's probably a good idea (I've always been surprised that this wasn't the way things worked), but maybe not for a minor release on the 1.4 branch. Alec _______________________________________________ Zope-CMF maillist - [email protected] http://mail.zope.org/mailman/listinfo/zope-cmf See http://collector.zope.org/CMF for bug reports and feature requests
