C optimisations in cAccessControl.c

Florent Guillaume <[email protected]>
Newsgroups gmane.comp.web.zope.coders
Message-ID <[email protected]>
Andreas, you checked this in:

http://cvs.zope.org/Zope/lib/python/AccessControl/Attic/cAccessControl.c.diff?r1=1.20.2.10&r2=1.20.2.11&hideattic=0&only_with_tag=Zope-2_7-branch

Log: replaced check for 'aq_' with strncmp()

--- Zope/lib/python/AccessControl/Attic/cAccessControl.c	2004/08/07 
17:15:48	1.20.2.10
+++ Zope/lib/python/AccessControl/Attic/cAccessControl.c	2004/12/11 
12:17:03	1.20.2.11
@@ -794,7 +794,7 @@
  	if ( PyString_Check(name) || PyUnicode_Check(name) ) {
  	    sname = PyString_AsString(name);
  	    if (sname != NULL) {
-		if (*sname == 'a' && sname[1]=='q' && sname[2]=='_') {
+        if (! strncmp(sname, "aq_", 3)) {
  			if (strcmp(sname,"aq_parent")   != 0 &&
                              strcmp(sname,"aq_inner") != 0 &&
                              strcmp(sname,"aq_explicit") != 0) {

My understanding is that the original code is much faster and does 
short-circuit evaluation. Why did you do this change ?

There is a comparable change in a later checkin that also was there for 
speed:

--- Zope/lib/python/AccessControl/Attic/cAccessControl.c	2004/12/11 
12:17:03	1.20.2.11
+++ Zope/lib/python/AccessControl/Attic/cAccessControl.c	2004/12/12 
10:52:15	1.20.2.12
@@ -1630,18 +1631,18 @@
  	/* we support both the old "_d" (from the Python implementation)
  	   and the new "__roles__"
  	*/
-	if (name_s[0] == '_') {
-		if (name_s[1] == '_') {
-			if (strcmp(name_s,"__name__") == 0)
-                                result= self->__name__;
-			else if (strcmp(name_s,"__roles__") == 0)
-                                result= self->__roles__;
-		}
-		else if (name_s[1] == 'p' && name_s[2] == 0)
-                                result= self->_p;
-		else if (name_s[1] == 'd' && name_s[2] == 0)
-                                result= self->__roles__;
+
+	if (name_s) {
+		if (!strcmp(name_s, "__name__"))
+            result= self->__name__;
+		else if (! strcmp(name_s, "__roles__"))
+            result= self->__roles__;
+		else if (! strcmp(name_s, "_p"))
+            result= self->_p;
+		else if (! strcmp(name_s, "_d"))
+            result= self->__roles__;
  	}
+
  	if (result) {
  		Py_INCREF(result);
  		return result;


The if (name_s[0] == '_') ... does short-circuit the common case of an 
attribute without '_' much faster than doing many string comparisons.


Also, when fixing the crash bug, you fixed the multiple calls to 
PyString_AsString(name) in SecurityManager_setattro but left those in 
SecurityManager_getattro.

The current diff is:
http://cvs.zope.org/Zope/lib/python/AccessControl/Attic/cAccessControl.c.diff?r1=text&tr1=1.20.2.10&r2=text&tr2=1.20.2.13&diff_format=h


I also noticed you haven't merged these changes to cAccessControl.c 
(crash bugfix) into svn trunk.

Florent

-- 
Florent Guillaume, Nuxeo (Paris, France)   CTO, Director of R&D
+33 1 40 33 71 59   http://nuxeo.com   [email protected]
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.