[Zope-Coders] C optimisations in cAccessControl.c

[Florent Guillaume]

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   fg at nuxeo.com