Improve f-string implementation: FORMAT_VALUE opcode

[ericvsmith]

BPO	25483
Nosy	@brettcannon, @larryhastings, @ericvsmith, @skrah, @markshannon, @berkerpeksag, @serhiy-storchaka
Files	format-opcode.diff format-opcode-1.diff format-opcode-2.diff format-opcode-3.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ericvsmith'
closed_at = <Date 2015-11-03.18:13:36.239>
created_at = <Date 2015-10-26.15:44:59.106>
labels = ['interpreter-core', 'type-feature']
title = 'Improve f-string implementation: FORMAT_VALUE opcode'
updated_at = <Date 2015-11-03.21:30:52.349>
user = 'https://github.com/ericvsmith'

bugs.python.org fields:

activity = <Date 2015-11-03.21:30:52.349>
actor = 'python-dev'
assignee = 'eric.smith'
closed = True
closed_date = <Date 2015-11-03.18:13:36.239>
closer = 'eric.smith'
components = ['Interpreter Core']
creation = <Date 2015-10-26.15:44:59.106>
creator = 'eric.smith'
dependencies = []
files = ['40863', '40864', '40880', '40928']
hgrepos = []
issue_num = 25483
keywords = ['patch']
message_count = 18.0
messages = ['253476', '253484', '253505', '253610', '253611', '253614', '253615', '253618', '253630', '253910', '253912', '253915', '253928', '254006', '254008', '254009', '254015', '254019']
nosy_count = 8.0
nosy_names = ['brett.cannon', 'larry', 'eric.smith', 'skrah', 'Mark.Shannon', 'python-dev', 'berker.peksag', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue25483'
versions = ['Python 3.6']

[ericvsmith]

Currently, the f-string f'a{3!r:10}' evaluates to bytecode that does the same thing as:

''.join(['a', format(repr(3), '10')])

That is, it literally calls the functions format() and repr(). The same holds true for str() and ascii() with !s and !a, respectively.

By redefining format, str, repr, and ascii, you can break or pervert the computation of the f-string's value:

>>> def format(v, fmt=None): return '42'
...
>>> f'{3}'
'42'

It's always been my intention to fix this. This patch adds an opcode FORMAT_VALUE, which instead of looking up format, etc., directly calls PyObject_Format, PyObject_Str, PyObject_Repr, and PyObject_ASCII. Thus, you can no longer modify what an f-string produces merely by overriding the named functions.

In addition, because I'm now saving the name lookups and function calls, performance is improved.

Here are the times without this patch:

$ ./python -m timeit -s 'x="test"' 'f"{x}"'
1000000 loops, best of 3: 0.3 usec per loop

$ ./python -m timeit -s 'x="test"' 'f"{x!s}"'
1000000 loops, best of 3: 0.511 usec per loop

$ ./python -m timeit -s 'x="test"' 'f"{x!r}"'
1000000 loops, best of 3: 0.497 usec per loop

$ ./python -m timeit -s 'x="test"' 'f"{x!a}"'
1000000 loops, best of 3: 0.461 usec per loop

And with this patch:

$ ./python -m timeit -s 'x="test"' 'f"{x}"'
10000000 loops, best of 3: 0.02 usec per loop

$ ./python -m timeit -s 'x="test"' 'f"{x!s}"'
100000000 loops, best of 3: 0.02 usec per loop

$ ./python -m timeit -s 'x="test"' 'f"{x!r}"'
10000000 loops, best of 3: 0.0896 usec per loop

$ ./python -m timeit -s 'x="test"' 'f"{x!a}"'
10000000 loops, best of 3: 0.0923 usec per loop

So a 90%+ speedup, for these simple cases.

Also, now f-strings are faster than %-formatting, at least for some types:

$ ./python -m timeit -s 'x="test"' '"%s"%x'
10000000 loops, best of 3: 0.0755 usec per loop

$ ./python -m timeit -s 'x="test"' 'f"{x}"'
10000000 loops, best of 3: 0.02 usec per loop

Note that people often "benchmark" %-formatting with code like the following. But the optimizer converts this to a constant string, so it's not a fair comparison:

$ ./python -m timeit '"%s"%"test"'
100000000 loops, best of 3: 0.0161 usec per loop

These microbenchmarks aren't the end of the story, since the string concatenation also takes some time. That's another optimization I might implement in the future.

Thanks to Mark and Larry for some advice on this.

[ericvsmith]

Small cleanups. Fixed a bug in PyCompile_OpcodeStackEffect.

[ericvsmith]

This patch addresses Larry's review, plus bumps the bytecode magic number.

[ericvsmith]

Oops. Forgot to include the diff with that last message. But it turns out it doesn't work, anyway, because I put the #define's in opcode.h, which is generated (so my code got deleted!).

I'll try to find some reasonable .h file to use and submit a new patch soon.

[brettcannon]

I know this issue is slowly turning into "make Eric update outdated docs", but if you find that https://docs.python.org/devguide/compiler.html#introducing-new-bytecode is outdated, could you update that doc?

[serhiy-storchaka]

I'll try to find some reasonable .h file to use and submit a new patch soon.

It's Lib/opcode.py.

[ericvsmith]

Brett: I'll take a look.

Serhiy: I'm looking for a place to put some #defines related to the bit masks and bit values that my FORMAT_VALUE opcode is using for opargs. One option is to just put them in Tools/scripts/generate_opcode_h.py, so that they end up in the generated opcode.h, but that seems a little sleazy. I can't find a better place they'd belong, though.

Specifically, I want to put these lines into a .h file to use by ceval.c and compile.c:

/* Masks and values for FORMAT_VALUE opcode. */
#define FVC_MASK      0x3
#define FVS_MASK      0x4
#define FVC_NONE      0x0
#define FVC_STR       0x1
#define FVC_REPR      0x2
#define FVC_ASCII     0x3
#define FVS_HAVE_SPEC 0x4

[serhiy-storchaka]

Does the dis module need these constants? If no, you can use either ceval.h or compile.h.

[ericvsmith]

Thanks, Serihy. I looked at those, and neither one is a great fit. But not having a better option, I went with ceval.h. Here's the patch.

[ericvsmith]

Some formatting improvements.

I removed one of the optimizations I was doing, because it's also done in PyObject_Format(). I plan on moving other optimizations into PyObject_Format(), but I'll open a separate issue for that.

I swapped the order of the parameters on the stack, so that I could use the micro-optimization of TOP() and SET_TOP().

I'll commit this shortly.

[serhiy-storchaka]

It looks to me that FVS_MASK and FVS_HAVE_SPEC are duplicates. FVS_HAVE_SPEC is set but FVS_MASK is tested.

[ericvsmith]

Right, they're the same because it's a single bit. You 'and' with a mask to get the bits you want, and you 'or' together the values. It's an old habit left over from my bit-twiddling days.

I guess the test could really be:

have_fmt_spec = (oparg & FVS_MASK) == FVS_HAVE_SPEC;

to make it more clear what I'm doing.

It's easier to see the same thing with the FVC_MASK and FVC_* values, since that field is multiple bits.