This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: 2to3 Fix_imports optimization
Type: performance Stage:
Components: 2to3 (2.x to 3.x conversion tool) Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: collinwinter Nosy List: benjamin.peterson, collinwinter, nedds
Priority: high Keywords: patch

Created on 2008-06-27 18:19 by nedds, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
pytree.py nedds, 2008-06-27 18:19 New version of pytree
fix_imports.py nedds, 2008-06-27 21:57 New version of fix_imports.py
fix_imports.diff nedds, 2008-07-01 17:54 Corrected changes
fix_imports.diff nedds, 2008-07-17 01:56 Diff for fix_imports, pytree, and test suite changes
Messages (12)
msg68840 - (view) Author: Nick Edds (nedds) Date: 2008-06-27 18:19
This is an optimization in pytree.py specifically for the bare_name
pattern from fix_imports.py. It also has the isinstance change I
previously suggested piggybacked onto it. Because the bare_name pattern
is so massive (764 nodes!), it is very slow to call _recursive_matches
on it. This fix has a special bare_name_matches fix that uses an
iterative matching solution specifically tailored to the bare_name
pattern. From preliminary testing, it appears to be roughly 25-30%
faster than the previous version of 2to3. If I uncomment the fix_imports
test, it fails 6 of them, but they are the same ones failed by the
current version of 2to3 and it fails them in the same way, so I think it
works. As with my previous isinstance chance, a one line change to
test_pytree is required.
msg68860 - (view) Author: Nick Edds (nedds) Date: 2008-06-27 21:57
Here are the changes we talked about to fix_imports.py which remove the
functionality for members. This causes a substantial performance boost,
as fix_imports was the main bottleneck for 2to3.
msg69051 - (view) Author: Collin Winter (collinwinter) * (Python committer) Date: 2008-07-01 17:15
The change to pytree.py doesn't add much speed benefit over the
fix_imports.py change, and causes a test to fail.

The fix_imports.py change, on the other hand, takes the test suite run
time from 8m31s down to 1m45s -- this needs to go in!

That said, I don't think the change is correct: you remove the portion
of the pattern that matches "from foo import bar, baz, x, y as z", as
well as the portion that matches "foo.bar" in the body of the code,
where foo is a module to be renamed. These should be added back, but
with support for member matching removed.
msg69055 - (view) Author: Nick Edds (nedds) Date: 2008-07-01 17:54
Here is a diff for the both the fix_imports changes which I corrected,
and the pytree changes. The pytree changes were a lot more significant
before the fix_imports change, but I think they are still a decent
improvement. I think I have now restored the functionality you wanted
without the members, although I'm not quite sure I made the patterns
exactly right. I tested the from a import b as c, as well as in text
foo.bar references, and they seemed to be corrected properly.
msg69635 - (view) Author: Collin Winter (collinwinter) * (Python committer) Date: 2008-07-14 01:11
fix_imports.diff fails to apply cleanly against HEAD of fix_imports.py.
Also, fix_imports2 now inherits from fix_imports, so it needs to be
fixed as well.


+         yield """power< module_name=%r 
+                  trailer<'.' import_as_names< any > > any* >
+               """ % old_module

Why are you using import_as_names here? That's meant to match a series
of "NAME [as NAME]" in import statements, where this part of the pattern
is meant to match usages of "urllib.foo()" in the code.
msg69655 - (view) Author: Nick Edds (nedds) Date: 2008-07-14 14:02
Yeah that import_as_names definitely shouldn't be there. I don't know
what I was thinking at the time, but that should just be an any I
believe. I'll clean this up today or tomorrow, update fix_imports2 as
well, and try to fix the tests for fix_imports.
msg69806 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-07-16 17:48
I've fixed the tests, so you can cross that one off your list. However,
the buildbots are now failing because lib2to3 takes too long to test.
How soon can we have this optimization applied?
msg69807 - (view) Author: Nick Edds (nedds) Date: 2008-07-16 17:52
I can hopefully have it all fixed up by tonight or tomorrow.
msg69855 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-07-16 23:28
Can we expect this in the next 2 hours? It's fine if not, I just need to
know whether the 2to3 tests should be disabled for the beta.
msg69861 - (view) Author: Nick Edds (nedds) Date: 2008-07-17 00:53
It should be done tonight, but probably not until around 11 central
time. Sorry for the delay.
msg69864 - (view) Author: Nick Edds (nedds) Date: 2008-07-17 01:56
Sorry I couldn't have this done earlier today. I updated the test suite,
and this is now passing all tests. Collin, could you verify that is has
all the functionality you were expecting? If the member functionality
turns out to actually be important for any of the modules in
fix_imports, it probably makes the most sense to break them out into
their own fixer. As an added note, I think there is still further room
to optimize fix_imports, specifically the very large pattern it still
generates, so I'm going to work on that at some point.
msg69865 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-07-17 02:05
Thanks very much for getting this done! I checked in the changes in
r65053, so they can make beta 2. I'm leaving the issue open, though, in
case Collin wants to make more changes.
History
Date User Action Args
2022-04-11 14:56:35adminsetgithub: 47468
2008-08-25 15:08:21benjamin.petersonsetstatus: open -> closed
resolution: accepted
2008-07-17 02:05:27benjamin.petersonsetmessages: + msg69865
2008-07-17 01:56:51neddssetfiles: + fix_imports.diff
messages: + msg69864
2008-07-17 00:53:19neddssetmessages: + msg69861
2008-07-16 23:28:15benjamin.petersonsetmessages: + msg69855
2008-07-16 17:52:51neddssetmessages: + msg69807
2008-07-16 17:48:23benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg69806
2008-07-14 14:02:51neddssetmessages: + msg69655
2008-07-14 01:11:22collinwintersetmessages: + msg69635
2008-07-13 23:35:22benjamin.petersonsetpriority: high
2008-07-01 17:54:27neddssetfiles: + fix_imports.diff
keywords: + patch
messages: + msg69055
2008-07-01 17:16:00collinwintersetmessages: + msg69051
2008-06-27 21:57:22neddssetfiles: + fix_imports.py
messages: + msg68860
2008-06-27 18:19:05neddscreate