[ZCM] [ZC] 1890/ 5 Comment "Minor TALES speedup"

[Collector: Zope Bugs, Features, and Patches ...]

Issue #1890 Update (Comment) "Minor TALES speedup"
 Status Resolved, Zope/feature+solution medium
To followup, visit:
  http://www.zope.org/Collectors/Zope/1890

==============================================================
= Comment - Entry #5 by chrism on Sep 26, 2005 3:59 am

The speed increase mentioned in the title of this wasn't intended to come from putting default value in the function arguments.  The patch didn't add any default keyword args, it just extended an existing default keyword arg value.

Default args as speedups make the code less readable and if they don't add any speed improvements they should be taken out, and I probably should have just done that.  Oh well.

But FWIW it took me about three hours yesterday to merge the (tiny) patch into the 3 active Zope branches including checking out all the various branches, running all the tests, accounting two different version control systems and adding a new simple test, not even including Zope 3.  I don't want to spend even half of that time again today.

Maybe there should be a "geddon" about the default args topic.
________________________________________
= Comment - Entry #4 by stevea on Sep 26, 2005 3:29 am

Is there any real point in using the "cache things from the module namespace in the arguments" thing nowadays in Python?  I think this hack has been made mostly obselete in recent Python releases.   I propose that any minor speed increase isn't worth the loss in legibility.
________________________________________
= Resolve - Entry #3 by chrism on Sep 25, 2005 9:57 am

 Status: Accepted => Resolved

Done on the 2.7, 2.8, and HEAD branches of Zope 2.  Not done in Zope 3.
________________________________________
= Comment - Entry #2 by ajung on Sep 24, 2005 6:35 am

Please merge for 2.8.2 and 2.7.8
________________________________________
= Request - Entry #1 by mcdonc on Sep 9, 2005 1:46 am

 Status: Pending => Accepted

 Supporters added: chrism


Uploaded:  "expression-render.diff"
 - http://www.zope.org/Collectors/Zope/1890/expression-render.diff/view
This patch to Products.PageTemplates.Expressions prevents its "render" function from being called for any "basic" Python type (avoiding the function call overhead and the very general sniffing logic in the render function body)... this is a minor speedup but it might save a few microseconds on every path expression call, and this can be important if path expression calls are part of the inner loop ofa rendering (which they oftetn are).  In one simple "real-world" page I observed that this patch reduced the function call count for "render" down from 57000 to 200 calls without any change in functionality.

The patch is inlined but also attached.  If no one complains, I will merge this patch into the Zope 2 HEAD (and the Zope 3 HEAD if I can figure out where its analogue is):

Index: Expressions.py
===================================================================
--- Expressions.py      (revision 38416)
+++ Expressions.py      (working copy)
@@ -156,7 +156,9 @@
         return 0
 
     def _eval(self, econtext,
-              isinstance=isinstance, StringType=type(''), render=render):
+              isinstance=isinstance,
+              BasicTypes=(str, unicode, dict, list, tuple, bool),
+              render=render):
         for expr in self._subexprs[:-1]:
             # Try all but the last subexpression, skipping undefined ones.
             try:
@@ -172,7 +174,7 @@
             if self._hybrid:
                 return ob
 
-        if self._name == 'nocall' or isinstance(ob, StringType):
+        if self._name == 'nocall' or isinstance(ob, BasicTypes):
             return ob
         # Return the rendered object
         return render(ob, econtext.vars)

==============================================================