-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
optimize bytecode for conditional branches #48965
Comments
This patch optimizes the bytecode for conditional branches. 1 0 BUILD_LIST 0 but after the patch it produces the following bytecode: 1 0 BUILD_LIST 0 Notice that not only the code is shorter, but the conditional jump Furthermore, the old jump opcodes (JUMP_IF_FALSE, JUMP_IF_TRUE) could
|
Here is an optional patch which provides the two opcodes I was talking A somewhat contrived example: "f = lambda x,y,z,v: (z if x else y) or v" Without the patch: 1 0 LOAD_FAST 0 (x) With the patch: 1 0 LOAD_FAST 0 (x) |
Those look nice, although I need to look at the patches in more detail. |
pybench runtimes (attached) are almost the same. The big win is on list |
I've backported condbranches-plus.patch to trunk, and I'm getting these PyBench: 1.84-2.21% faster PyBench was run with -w=1; 2to3 is translating its entire source I've haven't tested condbranches.patch vs condbranches-plus.patch; what I like these changes. Folding POP into JUMP_IF_{TRUE,FALSE} should have This patch mostly looks good, though you still need to change Doc/ |
Hello,
Thanks!
What is Spitfire?
condbranches.patch is the earlier version (without POP_OR_JUMP and
Well, the pure-Python compiler package doesn't exist in py3k, for which |
On Tue, Jan 13, 2009 at 3:25 PM, Antoine Pitrou <report@bugs.python.org> wrote:
http://code.google.com/p/spitfire/. It's a template system designed
I was mostly curious whether the POP_OR_JUMP and JUMP_OR_POP opcodes
I'll update the compiler package in 2.x and post my patch once I have |
Sorry, hadn't seen your message before removing the file. Here it is again. |
6% faster on a template system simply by optimizing conditional jumps is
Well, I don't remember really, although this is only a few weeks old. |
Looking through the patch... I don't really like the names for JUMP_OR_POP and POP_OR_JUMP. (They If the old opcodes weren't called JUMP_IF_XXX, I'd suggest the The comments in opcode.h and opcode.py are inconsistent. I now get a warning that PRED_POP_TOP is defined but not used, so you I wonder if BINARY_AND and BINARY_OR should get predictions ... not for I would probably put the POP_JUMP_IF_XXX definitions next to the You missed a comment referring to JUMP_IF_FALSE before POP_JUMP_IF_TRUE is only used in one place: assertions. I wonder if In compiler_comprehension_generator, "compiler_use_next_block(c, skip);" In peephole.c, s/JUMP_SIGN/JUMPS_ON_TRUE/ ? test_peepholer fails for me, which makes sense because it still refers Whoo, the peephole.c changes are complex. I'll do those in a second round. |
FWIW, the goal is to replace the opcode peephole optimizer with a more |
In peephole.c: _optimize isn't a great label name, but I don't have a great Your change to the "LOAD_CONST trueconst JUMP_IF_FALSE xx POP_TOP" I wonder what the "POP_TOP JUMP_FORWARD 1 POP_TOP" was ever for. Why did After "if (JUMP_SIGN(j) == JUMP_SIGN(opcode)) {", it might be nice to Otherwise looks good. |
Yes, this optimization seems meant for "while 1" and "while True" mainly
I'm as clueless as you... |
Le mercredi 14 janvier 2009 à 02:48 +0000, Jeffrey Yasskin a écrit :
I don't like them either, they're the only relatively short ones I could
With the patch, I don't think they need predictions anymore.
No, POP_JUMP_IF_TRUE is also used when optimizing the sequence
I'll look at this.
Great, another stupid name disappears :) |
I've updated Antoine's patch to incorporate my comments. This interacts |
Le mercredi 25 février 2009 à 00:51 +0000, Jeffrey Yasskin a écrit :
Doesn't sound crazier than doing it in another order :-)) 2459 defines its own JUMP_ABS_IF_TRUE / JUMP_ABS_IF_FALSE (which are the Thank you for taking this, Jeffrey, my own priority right now being the |
Committed as r69961. I'll post the backport to trunk here at least a day |
Oh, and no problem with picking up the patches. Thanks for writing them Here's the backport to trunk. I particularly enjoyed the bit in The patch is also #2 at http://codereview.appspot.com/20064. I'll post performance numbers as soon as I've measured them. |
The numbers are: Intel Core 2, gcc-4.3, 32-bit Django: PyBench: Pickle: Spitfire: Unpickle: Intel Core 2, gcc-4.3, 64-bit 2to3: Django: PyBench: Pickle: Spitfire: Unpickle: On my MacBook, gcc-4.0, 32-bit: Django: PyBench: Pickle: Spitfire: Unpickle: The performance isn't as good as I'd like, especially on 64-bits. I On the other hand, "./python.exe -m timeit -s 'x=range(500)' '[y+3 for y For py3k: For trunk: That list comprehension definitely takes advantage of skipping the |
Collin made some comments at http://codereview.appspot.com/20094. Here's |
Backported as r70071. I also fixed a couple things I missed in the py3k |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: