[issue26647] ceval: use Wordcode, 16-bit bytecode

Change by Serhiy Storchaka : -- pull_requests: -1041 ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Change by Alexander Belopolsky : -- components: +Interpreter Core -Documentation stage: needs patch -> resolved ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Donald Stufft : -- pull_requests: +1041 ___ Python tracker ___ ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: I left this issue open for documenting the wordcode. Now opened separate issue28810 for this. -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: This issue is done: see issue #27129 for the next step. -- dependencies: -Wordcode, part 2 resolution: -> fixed status: open -> closed ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Roundup Robot added the comment: New changeset 303cedfb9e7a by Victor Stinner in branch '3.6': Fix _PyGen_yf() https://hg.python.org/cpython/rev/303cedfb9e7a -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Ryan May : -- nosy: +Ryan May ___ Python tracker ___ ___ Python-bugs-list

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: Issue27129. -- components: -Interpreter Core dependencies: +Wordcode, part 2 stage: patch review -> needs patch ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: Serhiy: please open a new issue for your change. While it's related, it's different enough to deserve its own issue. By the way , please don't include generated importlib .h file in your patches. -- ___ Python

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Serhiy Storchaka : Added file: http://bugs.python.org/file43013/wordcode.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Serhiy Storchaka : Removed file: http://bugs.python.org/file43012/wordcode.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: Here is large patch (not including generated Python/importlib.h and Python/importlib_external.h). -- stage: needs patch -> patch review Added file: http://bugs.python.org/file43012/wordcode.patch ___ Python

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: I think we should make yet few related changes: * Change meaning of jump offsets. They should count not bytes, but code units (16-bit words). This will extend the range addressed by short commands (from 256 bytes to 256 words) and simplify ceval.c. * Change

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: Hi, I ran the CPython benchmark suite (my fork modified to be more stable) on ed4eec682199 (patched) vs 7a7f54fe0698 (base). The patched version contains wordcode (issue #26647) + 16-bit fetch for opcode and oparg (issue #27097). The speedup is quite nice.

[issue26647] ceval: use Wordcode, 16-bit bytecode

Nick Coghlan added the comment: I switched the target component to Documentation to reflect that as far as we know this is feature complete from a functional perspective, but there hasn't been a review of the docs for bytecode references yet, nor a decision on whether or not we want to

[issue26647] ceval: use Wordcode, 16-bit bytecode

Nick Coghlan added the comment: Chatting to Russell Keith-Magee, I realised the bytecode section in the devguide's description of the code generation pipeline may need some tweaks to account for the differences between 3.6 and earlier versions:

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: A documentation touch up for EXTENDED_ARG is included in #27095 -- ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: I join in the congratulations, Demur! Thank you for your contributions. I left this issue open for updating the documentation and other polishing. -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: > New changeset 3a57eafd8401 by Serhiy Storchaka in branch 'default': > Issue #26647: Python interpreter now uses 16-bit wordcode instead of bytecode. > https://hg.python.org/cpython/rev/3a57eafd8401 Yeah, congrats Demur! --

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: Oh, I forgot to add a note in What's New. And the documentation of the dis module should be updated (EXTENDED_ARG etc). Could anyone do this? -- stage: commit review -> needs patch ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Roundup Robot added the comment: New changeset 3a57eafd8401 by Serhiy Storchaka in branch 'default': Issue #26647: Python interpreter now uses 16-bit wordcode instead of bytecode. https://hg.python.org/cpython/rev/3a57eafd8401 -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: wpyD.patch LGTM, go ahead! We can still polish it later and discuss how to implement the 16-bit fetch ;-) It would be nice to add a short comment in Python/wordcode_helpers.h explaining that it contains code shared by the compiler and the peephole optimizer.

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: LGTM. If no one has more comments, I'm going to commit the patch. -- assignee: -> serhiy.storchaka stage: patch review -> commit review ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Removes 0 <= unsigned assertion & fixes j < 0 check to avoid overflow bug -- Added file: http://bugs.python.org/file42950/wpyD.patch ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: I have verified that wpyC does not produce signed/unsigned warnings with make DEBUG=1 -- Added file: http://bugs.python.org/file42949/wpyC.patch ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: Warnings still emitted in debug build. -- ___ Python tracker ___ ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Sorry for the nuisance of uploading another patch so soon. wpyB modifies test_ctypes now that __phello__ is smaller, & fixes a typo in a comment I made & removes a blank line I had added in when adding in if(0) logic -- Added file:

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Serhiy Storchaka : Added file: http://bugs.python.org/file42947/wpyA.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: I've tracked down the lnotab issue after modifying master/wpy to not vacuum NOPs. Old code puts LOAD_CONST at after NOPs, on the same line as return op, whereas new code is putting LOAD_CONST before NOPs, on it's own line I've attached a fix along with

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Compiler warnings can be fixed by using h instead of j in case RETURN_VALUE & casting CONST_LEN() to unsigned is safe As for the failing tests: I've finally figured out how to use Tools/freeze to fix __phello__ et al. I've brought up the trace test failure a few

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: test_importlib and test_trace are failed with wpy9.patch. -- ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: wpy9.patch looks mostly good to me, but there compiler warnings. -- ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Based on serhiy's LGTM I'm uploading hopefully final touches based on his feedback New in this are changes to Makefile.pre & PCbuild files. I can't really test PCbuild but I did test that make followed by modifying wordcode_helpers.h followed by make rebuilds

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: I regenerated wpy8.patch with Mercurial. I had to fix a minor conflict in Lib/importlib/_bootstrap_external.py. (I didn't read the patch.) -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by STINNER Victor : Added file: http://bugs.python.org/file42912/wpy8.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: Added new comments on Rietveld. -- stage: -> patch review ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: I've replaced wpy8 which I uploaded a few days ago as the previous version had a blank line removed from dis.rst by accident -- Added file: http://bugs.python.org/file42886/wpy8.patch ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Demur Rumed : Removed file: http://bugs.python.org/file42835/wpy8.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Demur Rumed : Added file: http://bugs.python.org/file42835/wpy8.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: Oh, sorry for misleading. I was going to continue reviewing peephole.c after found issues would addressed. In any case the patch needs to be synchronized with current sources (I expected that changes for modulefinder is no longer needed and changes for dis

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: I've been waiting on storchaka to finish code reviewing peephole based on his statement of 'Reviewing peephole.c still is not completed.' before uploading a patch which addresses the issues raised since -- ___ Python

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: > I should mention a change in dis which hasn't come up for the sake of > explicit discussion: I modified dis to not print EXTENDED_ARG. This may be a problem since jumps can now point on address not output in disassemble. May be this is good idea (I'm

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: Oh, I dislike the dis change. Please keep the current behaviour and open an issue to enhance the dis module. -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: I should mention a change in dis which hasn't come up for the sake of explicit discussion: I modified dis to not print EXTENDED_ARG. Instead one can tell by the byte indices having a hole (since they uniformly count up by 2 otherwise) & the argument value being

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Serhiy Storchaka : -- dependencies: +modulefinder should reuse the dis module ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Serhiy Storchaka added the comment: I found bugs in peephole.c. I have published my comments on Rietveld, but reviewing peephole.c is still not finished. -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Serhiy Storchaka : Added file: http://bugs.python.org/file42661/wpy7.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Plain git diff formatted patch. I installed mercurial but then `hg clone https://hg.python.org/cpython` failed twice over weird protocol errors (2nd one had to do with negative length) so I gave up -- Added file:

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: To create a patch accepted by Rietveld, you should clone the Mercurial repository, apply your patch, and then regenerate the diff using Mercurial: Something like: $ cd python_hg_clone $ patch -p1 < ../git_patch.patch $ hg diff > hg_diff.patch To clone: hg

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: I should be able to submit a wpy7.patch this evening, though I was never able to generate a patch in the format you prefer. Should I fall back to piping git diff? At this point it may be better if you take in the last suggestions as I'd probably end up removing

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: Demur: Serhiy and me reviewed your change wpy6.patch. Can you modify your change? If not, I can do it ;-) It's up to you. -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: @Serhiy and Yury: I'm waiting for your review on this change. I would like to push the wordcode change, I propose to push it sunday. Tell me if you need more time to review it. -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: I opened a thread on the python-dev mailing list to discuss wordcode: https://mail.python.org/pipermail/python-dev/2016-April/144044.html -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: I ran the Python benchmark suite on wpy6.patch. * My platform: Linux-4.4.4-301.fc23.x86_64-x86_64-with-fedora-23-Twenty_Three * My PC: CPU Intel i7-2600 (~2.9 GHz) with 12 GB of RAM * Benchmark ran on isolated CPU:

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: > Updated wpy5.patch to use a more standard diff format (patch generated with > Mercurial, hg diff > patch). Crap, I forgot Python/wordcode_helpers.h. I updated a fixed wpy6.patch. -- Added file: http://bugs.python.org/file42455/wpy6.patch

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by STINNER Victor : Removed file: http://bugs.python.org/file42454/wpy5.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: Updated wpy5.patch to use a more standard diff format (patch generated with Mercurial, hg diff > patch). -- Added file: http://bugs.python.org/file42454/wpy5.patch ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Made changes from code review, did a little extra on fixing up type consistency, not sure if this is exactly the patch format you wanted; I tried `git difftool --extcmd='diff -u' python/master` but it's listing the original files as being from /tmp I've updated

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: Demur Rumed: can you please rebase your patch? And can you please generate a patch without the git format? -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Roundup Robot added the comment: New changeset 7bf08a11d4c9 by Victor Stinner in branch 'default': Issue #26647: Cleanup opcode https://hg.python.org/cpython/rev/7bf08a11d4c9 New changeset 423e2a96189e by Victor Stinner in branch 'default': Issue #26647: Cleanup modulefinder

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: module_finder.patch: cleanup (optimize?) modulefinder.ModuleFinder.scan_opcodes_25(): Use an index rather than creating a lot of substrings. It's unrelated to Wordcode, it's just that I noticed the inefficient code while reviewing the whole patch.

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: [12:36] Could I get a code review for wordcode's 4th patchset? http://bugs.python.org/review/26647/#ps16875 ... [13:13] serprex: you'd be better off bumping the issue -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Got f_lasti working as -1. Applied PEP7. Unrelated: fixed a misnamed variable in test_grammar because it ran into a peephole bug (const -> jump_if_false erase didn't work when EXTENDED_ARGs were involved). dis has argval/arg set to None instead of the unused

[issue26647] ceval: use Wordcode, 16-bit bytecode

STINNER Victor added the comment: I reviewed wpy3.patch. I concur with Raymond, it's really nice to have a regular structure for the bytecode. -- Serhiy proposed to *reduce* the size of bytecode by adding new specialized bytecode which include the argument. For example (LOAD_CONST, 0) =>

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Demur Rumed : Added file: http://bugs.python.org/file42339/wpy3.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Demur Rumed : Removed file: http://bugs.python.org/file42338/wpy3.patch ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Addressed feedback from josh.rosenberg besides reintroducing FOURTH/SET_FOURTH -- Added file: http://bugs.python.org/file42338/wpy3.patch ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: Added back HAVE_ARGUMENT & HAS_ARG. As a result printing has removed arguments Removed some code which was labelled unrelated This does _not_ include having f_lasti be -1 instead of -2 -- Added file: http://bugs.python.org/file42331/wpy2.patch

[issue26647] ceval: use Wordcode, 16-bit bytecode

Brett Cannon added the comment: Thanks to Demur and Raymond for running the benchmarks. All of these numbers look good and with Raymond saying the code looks cleaner and everyone so far -- including me -- liking the overall idea this definitely seems worth continuing to work on. Thanks for

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by Josh Rosenberg : -- nosy: +josh.r ___ Python tracker ___ ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Raymond Hettinger added the comment: Report on Darwin Raymonds-2013-MacBook-Pro.local 15.4.0 Darwin Kernel Version 15.4.0: Fri Feb 26 22:08:05 PST 2016; root:xnu-3248.40.184~3/RELEASE_X86_64 x86_64 i386 Total CPU cores: 8 ### 2to3 ### Min: 4.680941 -> 4.437426: 1.05x faster Avg: 4.703692 ->

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: While it's good to know benchmarking in core Python goes beyond the microbenchmarks included in the distribution, I'm having some trouble with hg.python.org/benchmarks due to my system only having 256MB of ram I've attached results for 2 benchmarks: 2to3 & regex

[issue26647] ceval: use Wordcode, 16-bit bytecode

Raymond Hettinger added the comment: FWIW, I'm seeing about a 7% improvement to pystone. -- ___ Python tracker ___

[issue26647] ceval: use Wordcode, 16-bit bytecode

Raymond Hettinger added the comment: Demur, I think you're on the right track here. It will nice to be rid of the HAVE_ARGUMENT tests and to not have to decode the arguments one-byte at a time. Overall, the patch looks good (although it includes several small unrelated changes). Besides

[issue26647] ceval: use Wordcode, 16-bit bytecode

Brett Cannon added the comment: Thanks for the benchmark results, Demur, but I think the benchmarks Victor was talking about hg.Python.org/benchmarks -- ___ Python tracker

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: To clarify format of extended arg listings: 1st column is the number of instances of EXTENDED_ARG being emitted, 2nd column is length of bytecode, followed by filename The previous numbers were of modules, which generally are run-once and listing many

[issue26647] ceval: use Wordcode, 16-bit bytecode

Demur Rumed added the comment: I've attached some benchmarking results as requested There is 1 failing test which doesn't fail in master for test_trace; the unit test for #9936 -- Added file: http://bugs.python.org/file42324/wcpybm.txt ___ Python

[issue26647] ceval: use Wordcode, 16-bit bytecode

Changes by STINNER Victor : -- title: Wordcode -> ceval: use Wordcode, 16-bit bytecode ___ Python tracker ___