From 6c4e4981bc4e16a8d3b48f6463606fa1a7b36d3c Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 3 Apr 2020 09:35:08 +0000 Subject: [PATCH] [IMP] core: remove odoo.tools.safe_eval._get_opcodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's not necessary anymore as all supported Python versions implement get_instructions, and inlining the usage of that is as readable as calling _get_opcodes. Also use the subset/superset predicate for validity testing instead of difference as it's a fair bit faster: ❯ python3.8 -mtimeit -s 's1 = set(range(10)); s2 = set(range(5))' 's2 - s1' 5000000 loops, best of 5: 88.3 nsec per loop ❯ python3.8 -mtimeit -s 's1 = set(range(10)); s2 = set(range(5, 15))' 's2 - s1' 2000000 loops, best of 5: 159 nsec per loop ❯ python3.8 -mtimeit -s 's1 = set(range(10)); s2 = set(range(5))' 's1 >= s2' 5000000 loops, best of 5: 71.1 nsec per loop ❯ python3.8 -mtimeit -s 's1 = set(range(10)); s2 = set(range(5, 15))' 's1 >= s2' 5000000 loops, best of 5: 53.6 nsec per loop we're paying double in the failure case but that doesn't super duper matter because we're raising an exception and bailing out, the 24% gain on the happy path seems more relevant. closes odoo/odoo#48948 Signed-off-by: Xavier Morel (xmo) --- odoo/tools/safe_eval.py | 34 +++++----------------------------- 1 file changed, 5 insertions(+), 29 deletions(-) diff --git a/odoo/tools/safe_eval.py b/odoo/tools/safe_eval.py index 37b6e4b425f..460eccccae8 100644 --- a/odoo/tools/safe_eval.py +++ b/odoo/tools/safe_eval.py @@ -114,30 +114,6 @@ _SAFE_OPCODES = _EXPR_OPCODES.union(set(opmap[x] for x in [ _logger = logging.getLogger(__name__) -if hasattr(dis, 'get_instructions'): - def _get_opcodes(codeobj): - """_get_opcodes(codeobj) -> [opcodes] - - Extract the actual opcodes as an iterator from a code object - - >>> c = compile("[1 + 2, (1,2)]", "", "eval") - >>> list(_get_opcodes(c)) - [100, 100, 23, 100, 100, 102, 103, 83] - """ - return (i.opcode for i in dis.get_instructions(codeobj)) -else: - def _get_opcodes(codeobj): - i = 0 - byte_codes = codeobj.co_code - while i < len(byte_codes): - code = ord(byte_codes[i:i+1]) - yield code - - if code >= HAVE_ARGUMENT: - i += 3 - else: - i += 1 - def assert_no_dunder_name(code_obj, expr): """ assert_no_dunder_name(code_obj, expr) -> None @@ -181,11 +157,11 @@ def assert_valid_codeobj(allowed_codes, code_obj, expr): """ assert_no_dunder_name(code_obj, expr) - # almost twice as fast as a manual iteration + condition when loading - # /web according to line_profiler - codes = set(_get_opcodes(code_obj)) - allowed_codes - if codes: - raise ValueError("forbidden opcode(s) in %r: %s" % (expr, ', '.join(opname[x] for x in codes))) + # set operations are almost twice as fast as a manual iteration + condition + # when loading /web according to line_profiler + code_codes = {i.opcode for i in dis.get_instructions(code_obj)} + if not allowed_codes >= code_codes: + raise ValueError("forbidden opcode(s) in %r: %s" % (expr, ', '.join(opname[x] for x in (code_codes - allowed_codes)))) for const in code_obj.co_consts: if isinstance(const, CodeType):