Issue2268
Created on 2008-03-10 19:12 by belopolsky, last changed 2022-04-11 14:56 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| const-slice-folding.diff | belopolsky, 2008-03-10 19:12 | patch against revision 61203 | ||
| Messages (3) | |||
|---|---|---|---|
| msg63447 - (view) | Author: Alexander Belopolsky (belopolsky) * ![]() |
Date: 2008-03-10 19:12 | |
I am attaching a proof-of-concept patch which would optimize bytecode
generated from constant slices as follows:
Before patch:
>>> dis(lambda:x[1:2:3])
1 0 LOAD_GLOBAL 0 (x)
3 LOAD_CONST 0 (1)
6 LOAD_CONST 1 (2)
9 LOAD_CONST 2 (3)
12 BUILD_SLICE 3
15 BINARY_SUBSCR
16 RETURN_VALUE
After the patch:
>>> dis(lambda:x[1:2:3])
1 0 LOAD_GLOBAL 0 (x)
3 LOAD_CONST 3 (slice(1, 2, 3))
6 BINARY_SUBSCR
7 RETURN_VALUE
While the peephole optimizer changes are straightforward, the
optimization does not work unless slice objects gain hash and marshal
support.
While I don't see any problem with adding slice marshaling, the idea of
making slices hashable has recently been rejected (see issue1733184) and
I was supporting the rejection myself.
With this patch, however, I would like to reopen the discussion of
hash(slice(..)) issue.
Allowing constant folding for slices may tip the balance towards
allowing hash(slice(..)) assuming that {}[:] can still be prohibited.
One possible approach to this problem would be to emit a new bytecode
instead of BINARY_SUBSCR from slice syntax and have it reject mapping
objects. This should be easy for objects implemented in C, but for user
defined classes with custom __(get|set)item__ it may not be easy to
distinguish between a mapping and a sequence. However, I don't much of
a problem for always allowing x[:] for such types (user code can reject
slices if necessary).
If extra bytecode approach is taken, it is likely that d[slice(a,b)]
will end up being supported even if d[a:b] is not. Some may think it
would be a good feature, though.
A possible advantage of that approach would be a better error message
from an attempt to slice a dictionary. The current "unhashable type"
diagnostic is somewhat cryptic. "Cannot slice a dictionary" would be
much friendlier.
It is possible to work around unhashability of slices and still
implement folding, but the ideas that come to mind such as placing a
hashable subclass of slice into constants seem too artificial.
I am copying the "nosy" list from issue1733184 to start the discussion.
|
|||
| msg64564 - (view) | Author: Alexander Belopolsky (belopolsky) * ![]() |
Date: 2008-03-26 18:10 | |
Just to quantify the improvement: Before: $ ./python -m timeit -s"x='abc'" "x[::-1]" 1000000 loops, best of 3: 0.305 usec per loop $ ./python -O -m timeit -s"x='abc'" "x[::-1]" 1000000 loops, best of 3: 0.275 usec per loop After: $ ./python -m timeit -s"x='abc'" "x[::-1]" 1000000 loops, best of 3: 0.262 usec per loop $ ./python -O -m timeit -s"x='abc'" "x[::-1]" 1000000 loops, best of 3: 0.253 usec per loop For some reason, when I run pybench, the timings vary from run to run so much that I cannot even tell the difference. (Run to run differences are larger than patched to original.) FWIW, the micro-benchmark above shows 8% improvement with "-O" and 14% improvement without. |
|||
| msg114631 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2010-08-22 01:24 | |
The was a nice attempt at a peephole optimization. I'm rejecting it because: * too many other things need to be changed to support it * the measured win is somewhat small * we have a negative bias towards expanding the peephole optimizer * the AST may be a better place to do these kind of optimizations |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:56:31 | admin | set | github: 46521 |
| 2010-08-22 01:24:01 | rhettinger | set | status: open -> closed resolution: rejected messages: + msg114631 |
| 2010-08-22 01:20:38 | gvanrossum | set | nosy:
- gvanrossum |
| 2010-08-21 23:01:10 | georg.brandl | set | versions: + Python 3.2, - Python 3.0 |
| 2010-08-21 22:59:05 | georg.brandl | set | assignee: rhettinger |
| 2008-03-26 18:10:00 | belopolsky | set | messages: + msg64564 |
| 2008-03-10 19:18:18 | exarkun | set | nosy: - exarkun |
| 2008-03-10 19:12:36 | belopolsky | create | |
