[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Changes by Collin Winter coll...@gmail.com: -- nosy: +collinwinter ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Terry J. Reedy tjre...@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? Do you have another example in mind? -- nosy: +tjreedy ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Raymond Hettinger rhettin...@users.sourceforge.net added the comment:
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;
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6133
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Raymond Hettinger rhettin...@users.sourceforge.net added the comment: Attaching a rough patch for caching constant attribute lookups. Has an open TODO for adding the new name to co-co_varnames. -- Added file: http://bugs.python.org/file14115/constattr.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Martin v. Löwis mar...@v.loewis.de added the comment: Terry J. Reedy wrote: Terry J. Reedy tjre...@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. -- 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 justbe a LOAD_COST ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Terry J. Reedy tjre...@udel.edu added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Collin Winter coll...@gmail.com added the comment: On Fri, May 29, 2009 at 3:45 PM, Martin v. Löwis rep...@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. -- title: LOAD_CONST followed by LOAD_ATTR can be optimized to justbe a LOAD_COST - LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Antoine Pitrou pit...@free.fr added the comment: Does this optimization actually help in real-world cases? -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Raymond Hettinger rhettin...@users.sourceforge.net added the comment:
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).
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6133
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Raymond Hettinger rhettin...@users.sourceforge.net added the comment:
[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).
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6133
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Martin v. Löwis mar...@v.loewis.de added the comment: 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. -- 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 justbe a LOAD_COST ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Alex alex.gay...@gmail.com added the comment:
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).
--
Added file: http://bugs.python.org/file14105/python_const_fold.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6133
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Raymond Hettinger rhettin...@users.sourceforge.net added the comment:
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
--
assignee: - rhettinger
nosy: +rhettinger
type: - performance
versions: +Python 2.7, Python 3.2
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6133
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Alex alex.gay...@gmail.com added the comment: Fully functional. -- Added file: http://bugs.python.org/file14106/python_const_fold.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
New submission from Alex alex.gay...@gmail.com:
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.
--
components: Interpreter Core
messages: 88455
nosy: alex
severity: normal
status: open
title: LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue6133
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Alex alex.gay...@gmail.com added the comment: 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. -- keywords: +patch Added file: http://bugs.python.org/file14098/python_const_fold.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Alex alex.gay...@gmail.com added the comment: Small update so I don't change whitespace all over the plcae. -- Added file: http://bugs.python.org/file14099/python_const_fold.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Alex alex.gay...@gmail.com added the comment: Switch to using memset instead of a forloop. -- Added file: http://bugs.python.org/file14100/python_const_fold.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6133] LOAD_CONST followed by LOAD_ATTR can be optimized to just be a LOAD_COST
Alex alex.gay...@gmail.com added the comment: 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 :( -- Added file: http://bugs.python.org/file14101/python_const_fold.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6133 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
