Simplify and speed up interpreter for f-strings. Split FORMAT_VALUE opcode into CONVERT_VALUE, FORMAT_SIMPLE and FORMAT_WITH_SPEC.

markshannon · markshannon · commit 8ec15b34c063 · 2018-03-17T05:03:58.000-07:00
Compiler can then emit optimal sequence for each format expression.
diff --git a/Include/opcode.h b/Include/opcode.h
@@ -30,6 +30,8 @@ extern "C" {
 #define BINARY_TRUE_DIVIDE       27
 #define INPLACE_FLOOR_DIVIDE     28
 #define INPLACE_TRUE_DIVIDE      29
+#define FORMAT_SIMPLE            40
+#define FORMAT_WITH_SPEC         41
 #define GET_AITER                50
 #define GET_ANEXT                51
 #define BEFORE_ASYNC_WITH        52
@@ -119,14 +121,14 @@ extern "C" {
 #define BUILD_TUPLE_UNPACK      152
 #define BUILD_SET_UNPACK        153
 #define SETUP_ASYNC_WITH        154
-#define FORMAT_VALUE            155
 #define BUILD_CONST_KEY_MAP     156
 #define BUILD_STRING            157
 #define BUILD_TUPLE_UNPACK_WITH_CALL 158
 #define LOAD_METHOD             160
 #define CALL_METHOD             161
 #define CALL_FINALLY            162
 #define POP_FINALLY             163
+#define CONVERT_VALUE           164
 
 /* EXCEPT_HANDLER is a special, implicit block type which is created when
    entering an except handler. It is not an opcode but we define it here
diff --git a/Lib/dis.py b/Lib/dis.py
@@ -16,7 +16,7 @@
 _have_code = (types.MethodType, types.FunctionType, types.CodeType,
               classmethod, staticmethod, type)
 
-FORMAT_VALUE = opmap['FORMAT_VALUE']
+CONVERT_VALUE = opmap['CONVERT_VALUE']
 FORMAT_VALUE_CONVERTERS = (
     (None, ''),
     (str, 'str'),
@@ -347,13 +347,9 @@ def _get_instructions_bytes(code, varnames=None, names=None, constants=None,
                 argrepr = argval
             elif op in hasfree:
                 argval, argrepr = _get_name_info(arg, cells)
-            elif op == FORMAT_VALUE:
-                argval, argrepr = FORMAT_VALUE_CONVERTERS[arg & 0x3]
-                argval = (argval, bool(arg & 0x4))
-                if argval[1]:
-                    if argrepr:
-                        argrepr += ', '
-                    argrepr += 'with format'
+            elif op == CONVERT_VALUE:
+                argval = (None, str, repr, ascii)[arg]
+                argrepr = ('', 'str', 'repr', 'ascii')[arg]
             elif op == MAKE_FUNCTION:
                 argrepr = ', '.join(s for i, s in enumerate(MAKE_FUNCTION_FLAGS)
                                     if arg & (1<<i))
diff --git a/Lib/opcode.py b/Lib/opcode.py
@@ -84,6 +84,9 @@ def jabs_op(name, op):
 def_op('INPLACE_FLOOR_DIVIDE', 28)
 def_op('INPLACE_TRUE_DIVIDE', 29)
 
+def_op('FORMAT_SIMPLE', 40)
+def_op('FORMAT_WITH_SPEC', 41)
+
 def_op('GET_AITER', 50)
 def_op('GET_ANEXT', 51)
 def_op('BEFORE_ASYNC_WITH', 52)
@@ -203,7 +206,7 @@ def jabs_op(name, op):
 
 jrel_op('SETUP_ASYNC_WITH', 154)
 
-def_op('FORMAT_VALUE', 155)
+
 def_op('BUILD_CONST_KEY_MAP', 156)
 def_op('BUILD_STRING', 157)
 def_op('BUILD_TUPLE_UNPACK_WITH_CALL', 158)
@@ -213,4 +216,6 @@ def jabs_op(name, op):
 jrel_op('CALL_FINALLY', 162)
 def_op('POP_FINALLY', 163)
 
+def_op('CONVERT_VALUE', 164)
+
 del def_op, name_op, jrel_op, jabs_op
diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py
@@ -311,20 +311,22 @@ def _fstring(a, b, c, d):
 
 dis_fstring = """\
 %3d           0 LOAD_FAST                0 (a)
-              2 FORMAT_VALUE             0
+              2 FORMAT_SIMPLE
               4 LOAD_CONST               1 (' ')
               6 LOAD_FAST                1 (b)
               8 LOAD_CONST               2 ('4')
-             10 FORMAT_VALUE             4 (with format)
+             10 FORMAT_WITH_SPEC
              12 LOAD_CONST               1 (' ')
              14 LOAD_FAST                2 (c)
-             16 FORMAT_VALUE             2 (repr)
-             18 LOAD_CONST               1 (' ')
-             20 LOAD_FAST                3 (d)
-             22 LOAD_CONST               2 ('4')
-             24 FORMAT_VALUE             6 (repr, with format)
-             26 BUILD_STRING             7
-             28 RETURN_VALUE
+             16 CONVERT_VALUE            2 (repr)
+             18 FORMAT_SIMPLE
+             20 LOAD_CONST               1 (' ')
+             22 LOAD_FAST                3 (d)
+             24 CONVERT_VALUE            2 (repr)
+             26 LOAD_CONST               2 ('4')
+             28 FORMAT_WITH_SPEC
+             30 BUILD_STRING             7
+             32 RETURN_VALUE
 """ % (_fstring.__code__.co_firstlineno + 1,)
 
 def _g(x):
diff --git a/Python/ceval.c b/Python/ceval.c
@@ -70,6 +70,14 @@ static PyObject * special_lookup(PyObject *, _Py_Identifier *);
 static int check_args_iterable(PyObject *func, PyObject *vararg);
 static void format_kwargs_mapping_error(PyObject *func, PyObject *kwargs);
 
+typedef PyObject *(*convertion_func_ptr)(PyObject *);
+
+static const convertion_func_ptr CONVERTION_FUNCTIONS[4] = {
+    [FVC_STR] = PyObject_Str,
+    [FVC_REPR] = PyObject_Repr,
+    [FVC_ASCII] = PyObject_ASCII
+};
+
 #define NAME_ERROR_MSG \
     "name '%.200s' is not defined"
 #define UNBOUNDLOCAL_ERROR_MSG \
@@ -3284,60 +3292,47 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
             DISPATCH();
         }
 
-        TARGET(FORMAT_VALUE) {
-            /* Handles f-string value formatting. */
+        TARGET(CONVERT_VALUE) {
             PyObject *result;
-            PyObject *fmt_spec;
-            PyObject *value;
-            PyObject *(*conv_fn)(PyObject *);
-            int which_conversion = oparg & FVC_MASK;
-            int have_fmt_spec = (oparg & FVS_MASK) == FVS_HAVE_SPEC;
-
-            fmt_spec = have_fmt_spec ? POP() : NULL;
-            value = POP();
-
-            /* See if any conversion is specified. */
-            switch (which_conversion) {
-            case FVC_STR:   conv_fn = PyObject_Str;   break;
-            case FVC_REPR:  conv_fn = PyObject_Repr;  break;
-            case FVC_ASCII: conv_fn = PyObject_ASCII; break;
-
-            /* Must be 0 (meaning no conversion), since only four
-               values are allowed by (oparg & FVC_MASK). */
-            default:        conv_fn = NULL;           break;
-            }
-
-            /* If there's a conversion function, call it and replace
-               value with that result. Otherwise, just use value,
-               without conversion. */
-            if (conv_fn != NULL) {
-                result = conv_fn(value);
-                Py_DECREF(value);
-                if (result == NULL) {
-                    Py_XDECREF(fmt_spec);
-                    goto error;
-                }
-                value = result;
+            PyObject *value = POP();
+            convertion_func_ptr  conv_fn;
+            assert(oparg >= FVC_STR && oparg <= FVC_ASCII);
+            conv_fn = CONVERTION_FUNCTIONS[oparg];
+            result = conv_fn(value);
+            Py_DECREF(value);
+            if (result == NULL) {
+                goto error;
             }
+            PUSH(result);
+            DISPATCH();
+        }
 
-            /* If value is a unicode object, and there's no fmt_spec,
-               then we know the result of format(value) is value
-               itself. In that case, skip calling format(). I plan to
-               move this optimization in to PyObject_Format()
-               itself. */
-            if (PyUnicode_CheckExact(value) && fmt_spec == NULL) {
-                /* Do nothing, just transfer ownership to result. */
-                result = value;
-            } else {
-                /* Actually call format(). */
-                result = PyObject_Format(value, fmt_spec);
-                Py_DECREF(value);
-                Py_XDECREF(fmt_spec);
+        TARGET(FORMAT_SIMPLE) {
+            PyObject *value = TOP();
+            /* If value is a unicode object, then we know the result 
+             * of format(value) is value itself. */
+            if (!PyUnicode_CheckExact(value)) {
+                PyObject *result = PyObject_Format(value, NULL);
                 if (result == NULL) {
                     goto error;
                 }
+                SET_TOP(result);
+                Py_DECREF(value);
             }
+            DISPATCH();
+        }
 
+        TARGET(FORMAT_WITH_SPEC) {
+            PyObject *fmt_spec = POP();
+            PyObject *value = POP();
+            PyObject *result;
+            /* Call format(). */
+            result = PyObject_Format(value, fmt_spec);
+            Py_DECREF(value);
+            Py_DECREF(fmt_spec);
+            if (result == NULL) {
+                goto error;
+            }
             PUSH(result);
             DISPATCH();
         }
diff --git a/Python/compile.c b/Python/compile.c
@@ -1111,10 +1111,12 @@ stack_effect(int opcode, int oparg, int jump)
             return 1;
         case GET_YIELD_FROM_ITER:
             return 0;
-        case FORMAT_VALUE:
-            /* If there's a fmt_spec on the stack, we go from 2->1,
-               else 1->1. */
-            return (oparg & FVS_MASK) == FVS_HAVE_SPEC ? -1 : 0;
+        case CONVERT_VALUE:
+            return 0;
+        case FORMAT_WITH_SPEC:
+            return -1;
+        case FORMAT_SIMPLE:
+            return 0;
         case LOAD_METHOD:
             return 1;
         default:
@@ -3711,25 +3713,25 @@ compiler_formatted_value(struct compiler *c, expr_ty e)
 
     /* Evaluate the expression to be formatted. */
     VISIT(c, expr, e->v.FormattedValue.value);
-
-    switch (e->v.FormattedValue.conversion) {
-    case 's': oparg = FVC_STR;   break;
-    case 'r': oparg = FVC_REPR;  break;
-    case 'a': oparg = FVC_ASCII; break;
-    case -1:  oparg = FVC_NONE;  break;
-    default:
-        PyErr_SetString(PyExc_SystemError,
-                        "Unrecognized conversion character");
-        return 0;
+    if (e->v.FormattedValue.conversion != -1) {
+        switch (e->v.FormattedValue.conversion) {
+        case 's': oparg = FVC_STR;   break;
+        case 'r': oparg = FVC_REPR;  break;
+        case 'a': oparg = FVC_ASCII; break;
+        default:
+            PyErr_SetString(PyExc_SystemError,
+                            "Unrecognized conversion character");
+            return 0;
+        }
+        ADDOP_I(c, CONVERT_VALUE, oparg);
     }
     if (e->v.FormattedValue.format_spec) {
-        /* Evaluate the format spec, and update our opcode arg. */
+        /* Evaluate the format spec, and emit format opcode. */
         VISIT(c, expr, e->v.FormattedValue.format_spec);
-        oparg |= FVS_HAVE_SPEC;
+        ADDOP(c, FORMAT_WITH_SPEC);
+    } else {
+        ADDOP(c, FORMAT_SIMPLE);
     }
-
-    /* And push our opcode and oparg */
-    ADDOP_I(c, FORMAT_VALUE, oparg);
     return 1;
 }
 
diff --git a/Python/importlib.h b/Python/importlib.h
diff --git a/Python/importlib_external.h b/Python/importlib_external.h
diff --git a/Python/opcode_targets.h b/Python/opcode_targets.h
