[Zope3-checkins] SVN: Zope3/trunk/src/zope/security/ Shameless hack
to increase security checking performance
Jim Fulton
jim at zope.com
Tue Jun 22 18:40:23 EDT 2004
Log message for revision 25949:
Shameless hack to increase security checking performance
It is much faster to call operators, especially from C, than it is to
call methods. Now if a checker implements __setitem__, it will be
called rather than check or check_getattr. Similarly, if a checker
implements __getitem__, it will be called rather than proxy. Yes, this
is an egregious hack, but it does yield a significant speedup and is
thus worth it. Hopefully, it is well marked.
This change reduces the time required to display a sample contents
page by about 5%. This percentage will increase as other
optimizations are made and things get faster.
-=-
Modified: Zope3/trunk/src/zope/security/_proxy.c
===================================================================
--- Zope3/trunk/src/zope/security/_proxy.c 2004-06-22 21:08:29 UTC (rev 25948)
+++ Zope3/trunk/src/zope/security/_proxy.c 2004-06-22 22:40:23 UTC (rev 25949)
@@ -117,6 +117,11 @@
{
PyObject *r;
+ if (self->proxy_checker->ob_type->tp_as_mapping != NULL
+ && self->proxy_checker->ob_type->tp_as_mapping->mp_ass_subscript != NULL
+ && meth != str_check_setattr)
+ return self->proxy_checker->ob_type->tp_as_mapping->
+ mp_ass_subscript(self->proxy_checker, self->proxy.proxy_object, name);
r = PyObject_CallMethodObjArgs(self->proxy_checker, meth,
self->proxy.proxy_object, name,
@@ -130,8 +135,14 @@
#define PROXY_RESULT(self, result) \
if (result != NULL) { \
- PyObject *tmp = PyObject_CallMethodObjArgs(self->proxy_checker, str_proxy, \
- result, NULL); \
+ PyObject *tmp; \
+ if (self->proxy_checker->ob_type->tp_as_mapping != NULL \
+ && self->proxy_checker->ob_type->tp_as_mapping->mp_subscript != NULL) \
+ tmp = self->proxy_checker->ob_type->tp_as_mapping-> \
+ mp_subscript(self->proxy_checker, result); \
+ else \
+ tmp = PyObject_CallMethodObjArgs(self->proxy_checker, str_proxy, \
+ result, NULL); \
Py_DECREF(result); \
result = tmp; \
}
Modified: Zope3/trunk/src/zope/security/_zope_security_checker.c
===================================================================
--- Zope3/trunk/src/zope/security/_zope_security_checker.c 2004-06-22 21:08:29 UTC (rev 25948)
+++ Zope3/trunk/src/zope/security/_zope_security_checker.c 2004-06-22 22:40:23 UTC (rev 25949)
@@ -94,15 +94,12 @@
/* def check(self, object, name): */
-static PyObject *
-Checker_check(Checker *self, PyObject *args)
+static int
+Checker_check_int(Checker *self, PyObject *object, PyObject *name)
{
- PyObject *object, *name, *permission=NULL;
+ PyObject *permission=NULL;
int operator;
- if (!PyArg_ParseTuple(args, "OO", &object, &name))
- return NULL;
-
/* permission = self._permission_func(name) */
if (self->getperms)
permission = PyDict_GetItem(self->getperms, name);
@@ -113,11 +110,11 @@
/* if permission is CheckerPublic: */
/* return # Public */
if (permission == CheckerPublic)
- goto ok;
+ return 0;
if (checkPermission(permission, object, name) < 0)
- return NULL;
- goto ok;
+ return -1;
+ return 0;
}
@@ -131,9 +128,9 @@
/* return */
int ic = PySequence_Contains(_always_available, name);
if (ic < 0)
- return NULL;
+ return -1;
if (ic)
- goto ok;
+ return 0;
/* if name != '__iter__' or hasattr(object, name): */
/* __traceback_supplement__ = (TracebackSupplement, object) */
@@ -143,18 +140,32 @@
&& ! PyObject_HasAttr(object, name))
/* We want an attr error if we're asked for __iter__ and we don't
have it. We'll get one by allowing the access. */
- goto ok;
+ return 0;
}
-
- args = Py_BuildValue("OO", name, object);
- if (args != NULL)
- {
- PyErr_SetObject(ForbiddenAttribute, args);
- Py_DECREF(args);
- }
- return NULL;
- ok:
+ {
+ PyObject *args;
+ args = Py_BuildValue("OO", name, object);
+ if (args != NULL)
+ {
+ PyErr_SetObject(ForbiddenAttribute, args);
+ Py_DECREF(args);
+ }
+ return -1;
+ }
+}
+
+static PyObject *
+Checker_check(Checker *self, PyObject *args)
+{
+ PyObject *object, *name;
+
+ if (!PyArg_ParseTuple(args, "OO", &object, &name))
+ return NULL;
+
+ if (Checker_check_int(self, object, name) < 0)
+ return NULL;
+
Py_INCREF(Py_None);
return Py_None;
}
@@ -340,7 +351,14 @@
{NULL} /* Sentinel */
};
+static PyMappingMethods Checker_as_mapping = {
+ /* mp_length */ (inquiry)NULL,
+ /* mp_subscript */ (binaryfunc)Checker_proxy,
+ /* mp_ass_subscript */ (objobjargproc)Checker_check_int,
+};
+
+
static PyTypeObject CheckerType = {
PyObject_HEAD_INIT(NULL)
/* ob_size */ 0,
@@ -356,7 +374,7 @@
/* tp_repr */ (reprfunc)0,
/* tp_as_number */ 0,
/* tp_as_sequence */ 0,
- /* tp_as_mapping */ 0,
+ /* tp_as_mapping */ &Checker_as_mapping,
/* tp_hash */ (hashfunc)0,
/* tp_call */ (ternaryfunc)0,
/* tp_str */ (reprfunc)0,
Modified: Zope3/trunk/src/zope/security/checker.py
===================================================================
--- Zope3/trunk/src/zope/security/checker.py 2004-06-22 21:08:29 UTC (rev 25948)
+++ Zope3/trunk/src/zope/security/checker.py 2004-06-22 22:40:23 UTC (rev 25949)
@@ -426,16 +426,7 @@
except ForbiddenAttribute:
raise unauthorized_exception
- def check_getattr(self, object, name):
- 'See IChecker'
- try:
- Checker.check_getattr(self, object, name)
- except ForbiddenAttribute:
- self._checker2.check_getattr(object, name)
- except Unauthorized, unauthorized_exception:
- try: self._checker2.check_getattr(object, name)
- except ForbiddenAttribute:
- raise unauthorized_exception
+ check_getattr = __setitem__ = check
def check_setattr(self, object, name):
'See IChecker'
Modified: Zope3/trunk/src/zope/security/interfaces.py
===================================================================
--- Zope3/trunk/src/zope/security/interfaces.py 2004-06-22 21:08:29 UTC (rev 25948)
+++ Zope3/trunk/src/zope/security/interfaces.py 2004-06-22 22:40:23 UTC (rev 25949)
@@ -78,8 +78,16 @@
"""
def check_getattr(ob, name):
- """Check whether attribute access is allowed."""
+ """Check whether attribute access is allowed.
+ If a checker implements __setitem__, then __setitem__ will be
+ called rather than check_getattr to chack whether an attribute
+ access is allowed. This is a hack that allows significantly
+ greater performance due to the fact that low-level operator
+ access is much faster than method access.
+
+ """
+
def check_setattr(ob, name):
"""Check whether attribute assignment is allowed."""
@@ -88,12 +96,27 @@
The operation name is the Python special method name,
e.g. "__getitem__".
+
+ If a checker implements __setitem__, then __setitem__ will be
+ called rather than check to chack whether an operation is
+ allowed. This is a hack that allows significantly greater
+ performance due to the fact that low-level operator access is
+ much faster than method access.
+
"""
def proxy(value):
- """Return a security proxy for the value."""
+ """Return a security proxy for the value.
+ If a checker implements __getitem__, then __getitem__ will be
+ called rather than proxy to proxy the value. This is a hack
+ that allows significantly greater performance due to the fact
+ that low-level operator access is much faster than method
+ access.
+ """
+
+
class INameBasedChecker(IChecker):
"""Security checker that uses permissions to check attribute access."""
Modified: Zope3/trunk/src/zope/security/tests/test_proxy.py
===================================================================
--- Zope3/trunk/src/zope/security/tests/test_proxy.py 2004-06-22 21:08:29 UTC (rev 25948)
+++ Zope3/trunk/src/zope/security/tests/test_proxy.py 2004-06-22 22:40:23 UTC (rev 25949)
@@ -15,7 +15,7 @@
from zope.security.proxy import getChecker, ProxyFactory
from zope.proxy import ProxyBase as proxy, getProxiedObject
-class Checker:
+class Checker(object):
ok = 1
@@ -362,8 +362,69 @@
a, b = coerce(x, y)
self.failUnless(type(getProxiedObject(a)) is float and b is y)
+def test_using_mapping_slots_hack():
+ """The security proxy will use mapping slots, on the checker to go faster
+
+ If a checker implements normally, a checkers's check and
+ check_getattr methods are used to check operator and attribute
+ access:
+
+ >>> class Checker(object):
+ ... def check(self, object, name):
+ ... print 'check', name
+ ... def check_getattr(self, object, name):
+ ... print 'check_getattr', name
+ ... def proxy(self, object):
+ ... return 1
+ >>> def f():
+ ... pass
+ >>> p = ProxyFactory(f, Checker())
+ >>> p.__name__
+ check_getattr __name__
+ 1
+ >>> p()
+ check __call__
+ 1
+
+ But, if the checker has a __setitem__ method:
+
+ >>> def __setitem__(self, object, name):
+ ... print '__setitem__', name
+ >>> Checker.__setitem__ = __setitem__
+
+ It will be used rather than either check or check_getattr:
+
+ >>> p.__name__
+ __setitem__ __name__
+ 1
+ >>> p()
+ __setitem__ __call__
+ 1
+
+ If a checker has a __getitem__ method:
+
+ >>> def __getitem__(self, object):
+ ... return 2
+ >>> Checker.__getitem__ = __getitem__
+
+ It will be used rather than it's proxy method:
+
+ >>> p.__name__
+ __setitem__ __name__
+ 2
+ >>> p()
+ __setitem__ __call__
+ 2
+
+ """
+
+
+
def test_suite():
- return unittest.makeSuite(ProxyTests)
+ suite = unittest.makeSuite(ProxyTests)
+ from doctest import DocTestSuite
+ suite.addTest(DocTestSuite())
+ return suite
if __name__=='__main__':
from unittest import main
More information about the Zope3-Checkins
mailing list