From 20b6003dce87728e55be616e6eee2cbb162bc871 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Wed, 1 Apr 2015 15:10:08 +0200
Subject: [PATCH] [IMP] safe_eval: speed up evaluation

* creating the huge builtins dict inline represents ~40% of safe_eval's
  total cost according to line_profiler
* for the specific situation of opcodes, using a set difference is
  apparently much faster than for+if. all(gencomp) is also faster than
  for+if but not as much as letting Python do (almost) all of it

  might be that returning a set from _get_opcodes would be even faster,
  but didn't test it
---
 openerp/tools/safe_eval.py | 104 ++++++++++++++++++-------------------
 1 file changed, 52 insertions(+), 52 deletions(-)

diff --git a/openerp/tools/safe_eval.py b/openerp/tools/safe_eval.py
index 4eef310f68c..c30d39ef02c 100644
--- a/openerp/tools/safe_eval.py
+++ b/openerp/tools/safe_eval.py
@@ -144,10 +144,12 @@ def assert_valid_codeobj(allowed_codes, code_obj, expr):
                        is found in ``code_obj``
     """
     assert_no_dunder_name(code_obj, expr)
-    for opcode in _get_opcodes(code_obj):
-        if opcode not in allowed_codes:
-            raise ValueError(
-                "opcode %s not allowed (%r)" % (opname[opcode], expr))
+
+    # almost twice as fast as a manual iteration + condition when loading
+    # /web according to line_profiler
+    if set(_get_opcodes(code_obj)) - allowed_codes:
+        raise ValueError("forbidden opcode(s) in %r" % expr)
+
     for const in code_obj.co_consts:
         if isinstance(const, CodeType):
             assert_valid_codeobj(allowed_codes, const, 'lambda')
@@ -227,7 +229,44 @@ def _import(name, globals=None, locals=None, fromlist=None, level=-1):
     if name in _ALLOWED_MODULES:
         return __import__(name, globals, locals, level)
     raise ImportError(name)
-
+_BUILTINS = {
+    '__import__': _import,
+    'True': True,
+    'False': False,
+    'None': None,
+    'str': str,
+    'unicode': unicode,
+    'bool': bool,
+    'int': int,
+    'float': float,
+    'long': long,
+    'enumerate': enumerate,
+    'dict': dict,
+    'list': list,
+    'tuple': tuple,
+    'map': map,
+    'abs': abs,
+    'min': min,
+    'max': max,
+    'sum': sum,
+    'reduce': reduce,
+    'filter': filter,
+    'round': round,
+    'len': len,
+    'repr': repr,
+    'set': set,
+    'all': all,
+    'any': any,
+    'ord': ord,
+    'chr': chr,
+    'cmp': cmp,
+    'divmod': divmod,
+    'isinstance': isinstance,
+    'range': range,
+    'xrange': xrange,
+    'zip': zip,
+    'Exception': Exception,
+}
 def safe_eval(expr, globals_dict=None, locals_dict=None, mode="eval", nocopy=False, locals_builtins=False):
     """safe_eval(expression[, globals[, locals[, mode[, nocopy]]]]) -> result
 
@@ -245,12 +284,9 @@ def safe_eval(expr, globals_dict=None, locals_dict=None, mode="eval", nocopy=Fal
     :throws NameError: If the expression provided accesses forbidden names
     :throws ValueError: If the expression provided uses forbidden bytecode
     """
-    if isinstance(expr, CodeType):
+    if type(expr) is CodeType:
         raise TypeError("safe_eval does not allow direct evaluation of code objects.")
 
-    if globals_dict is None:
-        globals_dict = {}
-
     # prevent altering the globals/locals from within the sandbox
     # by taking a copy.
     if not nocopy:
@@ -260,55 +296,19 @@ def safe_eval(expr, globals_dict=None, locals_dict=None, mode="eval", nocopy=Fal
             _logger.warning(
                 "Looks like you are trying to pass a dynamic environment, "
                 "you should probably pass nocopy=True to safe_eval().")
-
-        globals_dict = dict(globals_dict)
+        if globals_dict is not None:
+            globals_dict = dict(globals_dict)
         if locals_dict is not None:
             locals_dict = dict(locals_dict)
 
-    globals_dict.update(
-        __builtins__={
-            '__import__': _import,
-            'True': True,
-            'False': False,
-            'None': None,
-            'str': str,
-            'unicode': unicode,
-            'bool': bool,
-            'int': int,
-            'float': float,
-            'long': long,
-            'enumerate': enumerate,
-            'dict': dict,
-            'list': list,
-            'tuple': tuple,
-            'map': map,
-            'abs': abs,
-            'min': min,
-            'max': max,
-            'sum': sum,
-            'reduce': reduce,
-            'filter': filter,
-            'round': round,
-            'len': len,
-            'repr': repr,
-            'set': set,
-            'all': all,
-            'any': any,
-            'ord': ord,
-            'chr': chr,
-            'cmp': cmp,
-            'divmod': divmod,
-            'isinstance': isinstance,
-            'range': range,
-            'xrange': xrange,
-            'zip': zip,
-            'Exception': Exception,
-        }
-    )
+    if globals_dict is None:
+        globals_dict = {}
+
+    globals_dict['__builtins__'] = _BUILTINS
     if locals_builtins:
         if locals_dict is None:
             locals_dict = {}
-        locals_dict.update(globals_dict.get('__builtins__'))
+        locals_dict.update(_BUILTINS)
     c = test_expr(expr, _SAFE_OPCODES, mode=mode)
     try:
         return eval(c, globals_dict, locals_dict)