Ideas to optimize this getitem/eval call?

Steven D'Aprano steve at REMOVE-THIS-cybersource.com.au
Sat Jan 3 01:16:39 EST 2009


On Fri, 02 Jan 2009 17:29:29 -0800, mario wrote:

> Hi,
> 
> below is the essence of a an expression evaluator, by means of a getitem
> lookup. The expression codes are compiled and cached -- the lookup is
> actually recursive, and the first time around it will always fail.
> 
> import sys
> class GetItemEval(object):
> 
>     def __init__(self):
>         self.globals = globals() # some dict (always the same)
>         self.locals = {} # some other dict (may be different between
> evaluations)
>         self.codes = {} # compiled code expressions cache
> 
>     def __getitem__(self, expr):
>         try:
>             return eval(self.codes[expr], self.globals, self.locals)

I was about to make a comment about this being a security hole, but I see 
from here 

http://evoque.gizmojo.org/usage/restricted/

that you are aware of at least some of the issues.

I must say though, your choice of builtins to prohibit seems rather 
arbitrary. What is dangerous about (e.g.) id() and isinstance()? 



>     except:
>         # KeyError, NameError, AttributeError, SyntaxError, 
>         # ValueError, TypeError, IOError

If you want to capture seven exceptions, then capture seven exceptions, 
not any exception.

You should write:

    except (KeyError, NameError, ..., IOError):

instead of a bare except clause. That will capture exceptions that 
represent bugs in your code as well as exceptions that should propbably 
be allowed to propagate, such as KeyboardInterupt and SystemExit.


>         # Special case if a KeyError is coming from the self.codes[name]
>         # lookup (traceback should consist of a single frame only):
>         if sys.exc_info()[2].tb_next is None:
>             if sys.exc_info()[0] is KeyError:
>                 self.codes[expr] = compile(expr, '<string>', 'eval')
>                     return self[expr]
>         # otherwise handle eval error in some way...

That seems awfully complicated for no good reason. If you want to handle 
KeyError differently, then capture KeyError:

    try:
        ...
    except KeyError:
        handle_keyerror()
    except: (NameError, ..., IOError):
        handle_everythingelse()



It also seems significant slower:

>>> import sys
>>> def catch1():
...     D = {}
...     try:
...         D['x']
...     except KeyError:
...         pass
...
>>> def catch2():
...     D = {}
...     try:
...         D['x']
...     except:
...         if sys.exc_info()[2].tb_next is None:
...             if sys.exc_info()[0] is KeyError:
...                 pass
...
>>> from timeit import Timer
>>> t1 = Timer('catch1()', 'from __main__ import catch1')
>>> t2 = Timer('catch2()', 'from __main__ import catch2, sys')
>>> min(t1.repeat())
4.8421750068664551
>>> min(t2.repeat())
6.2694659233093262



-- 
Steven



More information about the Python-list mailing list