msg88455 - (view) |
Author: Alex Gaynor (alex) * |
Date: 2009-05-27 23:19 |
Basically whenever you have a LOAD_CONST opcode, follwed by a LOAD_ATTR
you can replace both with a single LOAD_CONST. This optimizes things
like ", ".join or "{} {}".format (in my totally unscientific byte code
hackery it's about a 30% speedup on the loading the function). This can
be done in the peephole optimizer.
I'll try to work up a patch.
|
msg88460 - (view) |
Author: Alex Gaynor (alex) * |
Date: 2009-05-28 03:07 |
Here's my work so far, it seems to work as described. Except for the
fact that pyc creation with anything containign something with this
optimization will give a valueerror on bad marshall data, from trying to
marshall the method I assume.
|
msg88461 - (view) |
Author: Alex Gaynor (alex) * |
Date: 2009-05-28 03:17 |
Small update so I don't change whitespace all over the plcae.
|
msg88462 - (view) |
Author: Alex Gaynor (alex) * |
Date: 2009-05-28 03:23 |
Switch to using memset instead of a forloop.
|
msg88463 - (view) |
Author: Alex Gaynor (alex) * |
Date: 2009-05-28 04:23 |
I now *almost* have PyCFunctions marshalling, they seem to marhshall ok
but fail on unmarshalling. I think the whitespace stuff may have crept
back in, sorry :(
|
msg88474 - (view) |
Author: Alex Gaynor (alex) * |
Date: 2009-05-28 20:02 |
Optimization now works in the shell fine, and
marshal.loads(marshal.dumps(''.join)) works fine in the shell. However
when I try to run the tests the import of collections.namedtuple causes
the ValueError bad marshal data to appear, and I don't know why. Could
it have to do with the fact that namedtuple dynamically creates classes
(though I don't see how one of those could ever be the subject of
LOAD_CONST).
|
msg88483 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-05-28 21:51 |
-1. const.attr is not necessarily a constant. Indeed, "".join is *not* a
constant.
Furthermore, using this approach will lead to an endless series of types
to be added to marshalling, which is bad. For example, I believe that
current patch breaks on "".__class__
|
msg88484 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2009-05-28 22:13 |
I would like to see the current patch finished and recorded here for
reference. But I agree with Martin that changing marshal is a can of
worms. The peepholer is intended to be conservative and should avoid
anything that has some risk of being complicating our lives.
Another approach may be more successful. Here's a rough sketch:
in: LOAD_CONST 5 '{}' LOAD_ATTR 3 'format'
out: LOAD_CONST 8 ('{}','format') LOAD_CONST_ATTR 7.
def LOAD_CONST_ATTR(oparg):
# cache the lookup of '{}.format' in a fast local
x = GETLOCAL(oparg)
if (x == NULL)
UNPACK 2 --> obj, name
bm = getattr(obj, name)
STORE_FAST var
LOAD_FAST var
|
msg88486 - (view) |
Author: Alex Gaynor (alex) * |
Date: 2009-05-28 22:35 |
Fully functional.
|
msg88530 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2009-05-29 20:24 |
Martin, I agree that we would have to think carefully about all
attributes of all constants loaded by LOAD_CONST, and about
special-casing marshal, but given that "'str' object attribute 'join' is
read-only", how is ''.join not a constant? Do you have another example
in mind?
|
msg88531 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2009-05-29 20:28 |
I'm working a better patch now. Will give to collin to review when it's
ready. Here's a draft of the new opcode:
TARGET(LOAD_CONST_ATTR)
u = GETLOCAL(oparg); /* Cached attribute or NULL */
t = TOP(); /* t = (obj, name) where obj is constant */
if (u != NULL) { /* If cache is non-null, use it */
Py_INCREF(u);
SET_TOP(u);
Py_DECREF(t);
FAST_DISPATCH();
}
/* Cache is empty, do regular attribute lookup */
assert(PyTuple_CheckExact(t) && Py_SIZE(t) == 2);
v = PyTuple_GET_ITEM(t, 0);
w = PyTuple_GET_ITEM(t, 1);
x = PyObject_GetAttr(v, w);
Py_DECREF(t);
if (x != NULL) { /* Successful lookup; cache it and return */
SETLOCAL(oparg, x);
Py_INCREF(x);
SET_TOP(x);
break;
}
STACKADJ(-1); /* Attribute not found; goto err handler */
break;
|
msg88538 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2009-05-29 20:54 |
Attaching a rough patch for caching constant attribute lookups. Has an
open TODO for adding the new name to co->co_varnames.
|
msg88546 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-05-29 22:45 |
Terry J. Reedy wrote:
> Terry J. Reedy <tjreedy@udel.edu> added the comment:
>
> Martin, I agree that we would have to think carefully about all
> attributes of all constants loaded by LOAD_CONST, and about
> special-casing marshal, but given that "'str' object attribute 'join' is
> read-only", how is ''.join not a constant?
py> s = ""
py> s.join is s.join
False
Every time you read it, you get a new object. Not what I would call a
constant. If you don't see how this matters, try
def foo():
return "".join
print foo() is foo()
with and without your patch.
|
msg88552 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2009-05-29 23:22 |
You are right, of course: bound methods are currently created fresh on
each access, even though each is equal except for identity. I was
thinking in terms of
>>> str.join is str.join
True
This appears to be a classic time-space tradeoff: caching bound methods,
which is what I understand Raymond's patch does (when the attribute is a
method), saves time (with enough reuses) but takes space that could
gradually grow.
The reply to the OP could be that people who care about time should use
the standard idiom of explicitly savingthe bound method:
>>> bjoin = ''.join
...
On the other hand, such bindings look a bit messy and tend to be local.
|
msg88556 - (view) |
Author: Collin Winter (collinwinter) * |
Date: 2009-05-30 00:04 |
On Fri, May 29, 2009 at 3:45 PM, Martin v. Löwis <report@bugs.python.org> wrote:
> py> s = ""
> py> s.join is s.join
> False
>
> Every time you read it, you get a new object. Not what I would call a
> constant. If you don't see how this matters, try
>
> def foo():
> return "".join
>
> print foo() is foo()
>
> with and without your patch.
The fact that it returns a new object every time seems like an
implementation detail, and one that users find confusing (I know I
once filed a bug about it).
One problem I recognize is that the proposed patch would presumably
create an asymmetry between "x".join is "x".join and "x = 'x'; x.join
is x.join" where the former would evaluate to True and the latter to
False. That seems surmountable, though.
|
msg88558 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2009-05-30 00:09 |
Does this optimization actually help in real-world cases?
|
msg88561 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2009-05-30 03:09 |
Returning the same object vs new object for bound methods is a
non-guaranteed implementation detail (as long the other semantics remain
true). I think Martin's real concern is that trying to intern bound
methods would be a can of worms (one that I wouldn't want to open).
FWIW, str.join ignored by this patch. The "str" builtin is an
overridable builtin (LOAD_GLOBAL "str" LOAD_ATTR "join"). In contrast,
",".join or "{}".format does not have dynamically changeable behavior
(LOAD_CONST "{}" LOAD_ATTR "format").
|
msg88562 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2009-05-30 03:14 |
[AP]
> Does this optimization actually help in real-world cases?
Yes and no. Yes, there are real world cases like ','.join and
'{}'.format that are dramatically sped-up. No, there are probably no
real-world programs that are sped-up significantly in their entirety --
no one optimization will ever do that (Amdahl's law). Pretty much
anytime the substitution gets made there is a savings on second and
subsequent calls (just like any form of caching).
|
msg88563 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-05-30 05:17 |
> Returning the same object vs new object for bound methods is a
> non-guaranteed implementation detail (as long the other semantics remain
> true). I think Martin's real concern is that trying to intern bound
> methods would be a can of worms (one that I wouldn't want to open).
I'm also concerned about the change in behavior. Whether or not it is
guaranteed - some code in the world *will* notice. IMO, optimizations
should only be implemented if there is no change in observable behavior,
or if the improvement is so large that breaking compatibility is
justified.
|
msg115349 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2010-09-02 00:43 |
This proposal hasn't gotten much love or support and I'm no longer interested in it. Aside from ''.join and '{}'.format, there doesn't seem to be many common cases, so there's no big win here.
|
msg115350 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2010-09-02 01:25 |
What's the point of '{}'.format() anyway given the format() builtin?
|
msg221022 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-06-19 21:15 |
Given the last two comments can this be closed as won't fix?
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:49 | admin | set | github: 50383 |
2014-06-20 01:34:01 | terry.reedy | set | status: open -> closed resolution: later -> rejected stage: resolved |
2014-06-19 21:15:34 | BreamoreBoy | set | nosy:
+ BreamoreBoy messages:
+ msg221022
|
2010-09-02 01:25:18 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages:
+ msg115350
|
2010-09-02 00:43:52 | rhettinger | set | assignee: rhettinger -> resolution: later messages:
+ msg115349 versions:
+ Python 3.3, - Python 3.2 |
2010-04-03 03:20:48 | rhettinger | set | priority: low versions:
- Python 2.7 |
2009-08-21 16:00:18 | serprex | set | title: LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST -> LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_CONST |
2009-05-30 05:18:10 | loewis | set | messages:
+ msg88563 title: LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST -> LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST |
2009-05-30 03:14:47 | rhettinger | set | messages:
+ msg88562 |
2009-05-30 03:09:47 | rhettinger | set | messages:
+ msg88561 |
2009-05-30 00:09:38 | pitrou | set | nosy:
+ pitrou messages:
+ msg88558
|
2009-05-30 00:04:41 | collinwinter | set | messages:
+ msg88556 title: LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST -> LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST |
2009-05-29 23:22:19 | terry.reedy | set | messages:
+ msg88552 |
2009-05-29 22:45:37 | loewis | set | messages:
+ msg88546 title: LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST -> LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST |
2009-05-29 20:54:46 | rhettinger | set | files:
+ constattr.diff
messages:
+ msg88538 |
2009-05-29 20:29:00 | rhettinger | set | messages:
+ msg88531 |
2009-05-29 20:24:59 | terry.reedy | set | nosy:
+ terry.reedy messages:
+ msg88530
|
2009-05-29 16:27:47 | collinwinter | set | nosy:
+ collinwinter
|
2009-05-28 22:35:51 | alex | set | files:
+ python_const_fold.diff
messages:
+ msg88486 |
2009-05-28 22:13:26 | rhettinger | set | versions:
+ Python 2.7, Python 3.2 nosy:
+ rhettinger
messages:
+ msg88484
assignee: rhettinger type: performance |
2009-05-28 21:51:26 | loewis | set | nosy:
+ loewis messages:
+ msg88483
|
2009-05-28 20:02:27 | alex | set | files:
+ python_const_fold.diff
messages:
+ msg88474 |
2009-05-28 04:23:17 | alex | set | files:
+ python_const_fold.diff
messages:
+ msg88463 |
2009-05-28 03:23:26 | alex | set | files:
+ python_const_fold.diff
messages:
+ msg88462 |
2009-05-28 03:17:30 | alex | set | files:
+ python_const_fold.diff
messages:
+ msg88461 |
2009-05-28 03:07:40 | alex | set | files:
+ python_const_fold.diff keywords:
+ patch messages:
+ msg88460
|
2009-05-27 23:19:50 | alex | create | |