[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Roundup Robot]

Roundup Robot added the comment:

New changeset 1ddeb2e175df by Eric V. Smith in branch 'default':
Issue 25483: Add an opcode to make f-string formatting more robust.
https://hg.python.org/cpython/rev/1ddeb2e175df

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Roundup Robot]

Roundup Robot added the comment:

New changeset 4734713a31ed by Eric V. Smith in branch 'default':
Issue 25483: Update dis.rst with FORMAT_VALUE opcode description.
https://hg.python.org/cpython/rev/4734713a31ed

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

Eric V. Smith added the comment:

Brett: https://docs.python.org/devguide/compiler.html#introducing-new-bytecode 
looks correct (and reminded me to update dis.rst!).

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Roundup Robot]

Roundup Robot added the comment:

New changeset 93fd7adbc7dd by Eric V. Smith in branch 'default':
Issue 25483: Fix doc typo and added versionadded. Thanks, Berker Peksag.
https://hg.python.org/cpython/rev/93fd7adbc7dd

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Berker Peksag]

Berker Peksag added the comment:

+   * ``(flags & 0x03) == 0x00``: *value* is formattedd as-is.

Just noticed a small typo: formattedd

Also, needs ``.. versionadded:: 3.6``

--
nosy: +berker.peksag

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

Eric V. Smith added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

Eric V. Smith added the comment:

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.

--
Added file: http://bugs.python.org/file40928/format-opcode-3.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Stefan Krah]

Stefan Krah added the comment:

The MASK idiom is nice and I think it's good to be exposed to
it from time to time.

--
nosy: +skrah

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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

It's Lib/opcode.py.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

Eric V. Smith added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Brett Cannon]

Brett Cannon added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

Eric V. Smith added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

Eric V. Smith added the comment:

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.

--
Added file: http://bugs.python.org/file40880/format-opcode-2.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Brett Cannon]

Changes by Brett Cannon :


--
nosy: +brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

Eric V. Smith added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

New submission from Eric V. Smith:

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}"'
100 loops, best of 3: 0.3 usec per loop

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

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

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


And with this patch:

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

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

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

$ ./python -m timeit -s 'x="test"' 'f"{x!a}"'
1000 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'
1000 loops, best of 3: 0.0755 usec per loop

$ ./python -m timeit -s 'x="test"' 'f"{x}"'
1000 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"'
1 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.

--
assignee: eric.smith
components: Interpreter Core
files: format-opcode.diff
keywords: patch
messages: 253476
nosy: Mark.Shannon, eric.smith, larry
priority: normal
severity: normal
stage: patch review
status: open
title: Improve f-string implementation: FORMAT_VALUE opcode
type: enhancement
versions: Python 3.6
Added file: http://bugs.python.org/file40863/format-opcode.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25483] Improve f-string implementation: FORMAT_VALUE opcode

[Eric V. Smith]

Eric V. Smith added the comment:

Small cleanups. Fixed a bug in PyCompile_OpcodeStackEffect.

--
Added file: http://bugs.python.org/file40864/format-opcode-1.diff

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com