[issue26219] implement per-opcode cache in ceval

Change by Eric Snow : -- nosy: +eric.snow ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue26219] implement per-opcode cache in ceval

Inada Naoki added the comment: On Mon, Oct 7, 2019 at 9:41 PM Mark Shannon wrote: > > Mark Shannon added the comment: > > Given that > def foo(): int; str; bytes; float; int; str; bytes; float > can be trivially be rewritten as > def foo(): pass > I think that benchmark is meaningless. > Do

[issue26219] implement per-opcode cache in ceval

Brett Cannon added the comment: I personally think it would be fine to change the wording to say "measurable speed-up" and not attribute a specific number. -- ___ Python tracker

[issue26219] implement per-opcode cache in ceval

Mark Shannon added the comment: Given that def foo(): int; str; bytes; float; int; str; bytes; float can be trivially be rewritten as def foo(): pass I think that benchmark is meaningless. I really don't think we should be making claims like "40% speedup" for such contrived examples. It

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: > What's the evidence for the claimed speedup? > What's the speedup on the benchmark suite? https://github.com/python/cpython/pull/12884#issuecomment-485091040 points to:

[issue26219] implement per-opcode cache in ceval

Mark Shannon added the comment: "What's new in Python 3.8" says that this change speeds up the LOAD_GLOBAL opcode by 40%. That seems meaningless as a program cannot have LOAD_GLOBAL in isolation so any test would have other bytecodes as well. What's the evidence for the claimed speedup?

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: New changeset 996e52623af3854552d41751e0c2522bc0a7e84f by Victor Stinner (Miss Islington (bot)) in branch '3.8': bpo-26219: Fix compiler warning in _PyCode_InitOpcache() (GH-13997) (GH-14000)

[issue26219] implement per-opcode cache in ceval

Change by miss-islington : -- pull_requests: +13863 pull_request: https://github.com/python/cpython/pull/14000 ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: New changeset 376ce9852eec4e97745c723f0dd0fe64383c6cd3 by Victor Stinner in branch 'master': bpo-26219: Fix compiler warning in _PyCode_InitOpcache() (GH-13997) https://github.com/python/cpython/commit/376ce9852eec4e97745c723f0dd0fe64383c6cd3 --

[issue26219] implement per-opcode cache in ceval

Change by STINNER Victor : -- pull_requests: +13860 pull_request: https://github.com/python/cpython/pull/13997 ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: I understand that the initial issue (add an opcode cache) has been implemented. Well done Yury and INADA-san! Please open new issues for follow-up like the new optimization ideas ;-) -- resolution: -> fixed stage: patch review -> resolved status:

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: New changeset ea9f168957f7236aae2185f65e0dc608a9a7a37b by Victor Stinner in branch 'master': bpo-26219: Fix compiler warning in _PyCode_InitOpcache() (GH-13809) https://github.com/python/cpython/commit/ea9f168957f7236aae2185f65e0dc608a9a7a37b --

[issue26219] implement per-opcode cache in ceval

Change by STINNER Victor : -- pull_requests: +13693 pull_request: https://github.com/python/cpython/pull/13809 ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: The opcode cache introduced an issue in reference leak hunting: see bpo-37146. -- ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

Inada Naoki added the comment: New changeset 395420e2a35937fa9777fc3c8b0086629db95e27 by Inada Naoki in branch 'master': bpo-26219: remove unused code (GH-13775) https://github.com/python/cpython/commit/395420e2a35937fa9777fc3c8b0086629db95e27 --

[issue26219] implement per-opcode cache in ceval

Change by Inada Naoki : -- pull_requests: +13662 pull_request: https://github.com/python/cpython/pull/13775 ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

Inada Naoki added the comment: I committed cache only for LOAD_GLOBAL, which is much simpler than LOAD_ATTR or LOAD_METHOD. Caches for other instructions will be implemented 3.9 or later. -- ___ Python tracker

[issue26219] implement per-opcode cache in ceval

Inada Naoki added the comment: New changeset 91234a16367b56ca03ee289f7c03a34d4cfec4c8 by Inada Naoki in branch 'master': bpo-26219: per opcode cache for LOAD_GLOBAL (GH-12884) https://github.com/python/cpython/commit/91234a16367b56ca03ee289f7c03a34d4cfec4c8 --

[issue26219] implement per-opcode cache in ceval

Change by Inada Naoki : -- pull_requests: +12809 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: > Would it be OK if we add this to 3.7beta2? IMHO we need more time than beta1=>beta2 window to review properly the design, test the implementation, fix any potential regression, etc. Such change is perfect for the *start of a

[issue26219] implement per-opcode cache in ceval

Ned Deily added the comment: I concur with Guido and Nick; let's target this for 3.8. -- ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

Guido van Rossum added the comment: It's up to the release manager, but personally it feels like you're pushing too hard. Since you've done that frequently in the past I think we should help you by declining your request. 3.8 is right around the corner (on a Python release

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: > It's up to the release manager, but personally it feels like you're pushing too hard. Since you've done that frequently in the past I think we should help you by declining your request. NP, I totally understand. > Now maybe we'll need

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: > With the 3.7 beta deadline just around the corner, 3.8 will be the next > opportunity to reconsider this idea. Nick, Ned, Guido, Would it be OK if we add this to 3.7beta2? I feel kind of bad about this one... few thoughts: 1. The

[issue26219] implement per-opcode cache in ceval

Nick Coghlan added the comment: With the 3.7 beta deadline just around the corner, 3.8 will be the next opportunity to reconsider this idea. -- versions: +Python 3.8 -Python 3.6 ___ Python tracker

[issue26219] implement per-opcode cache in ceval

Changes by Jim Fasarakis-Hilliard : -- nosy: +Jim Fasarakis-Hilliard ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

Changes by Roy Williams : -- nosy: +Roy Williams ___ Python tracker ___ ___

[issue26219] implement per-opcode cache in ceval

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

[issue26219] implement per-opcode cache in ceval

Changes by Jakub Stasiak : -- nosy: +jstasiak ___ Python tracker ___ ___

[issue26219] implement per-opcode cache in ceval

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

[issue26219] implement per-opcode cache in ceval

Guido van Rossum added the comment: All sounds good. Glad the issue of long-running loops is at least on your radar. -- ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: > OK, I get it. I think it would be really helpful if issue 26110 was updated, > reviewed and committed -- it sound like a good idea on its own, and it needs > some burn-in time due to the introduction of two new opcodes. (That's > especially important since

[issue26219] implement per-opcode cache in ceval

Guido van Rossum added the comment: OK, I get it. I think it would be really helpful if issue 26110 was updated, reviewed and committed -- it sound like a good idea on its own, and it needs some burn-in time due to the introduction of two new opcodes. (That's especially important since

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: > I'm confused by the relationship between this and issue 26110. This patch embeds the implementation of 26110. I'm no longer sure it was a good idea to have two issues instead of one, everybody seems to be confused about that ;) > That seems to be a much

[issue26219] implement per-opcode cache in ceval

Guido van Rossum added the comment: I'm also looking for some example code that would show clearly the kind of speedup we're talking about. -- ___ Python tracker

[issue26219] implement per-opcode cache in ceval

Guido van Rossum added the comment: I'm confused by the relationship between this and issue 26110. That seems to be a much simpler patch (which also doesn't apply cleanly). If 26110 really increases method calls by 20%, what does this add? (By which I mean (a) what additional optimizations

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: Alright, attaching a rebased patch (opcache3.patch). Some caveats: 1. The patch embeds a fairly outdated PEP 509 implementation. 2. A PoC implementation of LOAD_METHOD opcode that should be really cleaned-up (and some parts of it rewritten). 3. I was going

[issue26219] implement per-opcode cache in ceval

Guido van Rossum added the comment: Thanks, that's a cool stat. Please do update the patch. -- ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: Hi Guido, I'll try to update the patch soon. > but I worry that there's a catch (e.g. memory cost that doesn't become > apparent until you have a large program, or minimal speedup on realistic > code). Here's an excerpt from my email [1] to Python-dev on

[issue26219] implement per-opcode cache in ceval

Guido van Rossum added the comment: Victor brought this patch to my attention as the motivation for PEP 509. Unfortunately the patch doesn't apply cleanly and I don't have time to try and piece together all the different parts. From the description on python-dev you linked to there are

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: See the issue #26647 which may make the implementation of this cache simpler. See also my message about inline caching: https://bugs.python.org/issue26647#msg262622 -- ___ Python tracker

[issue26219] implement per-opcode cache in ceval

Francis MB added the comment: >From the two checks on Python/compile.c: + expr_ty meth = e->v.Call.func; [...] +/* Check [...] that + the call doesn't have keyword parameters. */ [...] +/* Check that there are no *varargs types of arguments. */ [...] I just wonder how many times

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: > From the two checks on Python/compile.c: Stuff done in compile.c is cached in *.pyc files. The for-loop you saw shouldn't take a lot of time - it iterates through function parameters, which an average function doesn't have more than 3-6. --

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: Victor, Thanks for the initial review. I'll work on the patch sometime later next week. As for test_vicious_descriptor_nonsense -- yeah, I saw that too. I know what's going on there and I know how to fix that. FWIW that test tests a very

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: I'm concerned by the test_descr failure. == ERROR: test_vicious_descriptor_nonsense (test.test_descr.ClassPropertiesAndMethods)

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: Attaching a new version of the patch. Issues #26058 and #26110 need to be merged before we can start reviewing it. -- Added file: http://bugs.python.org/file41776/opcache2.patch ___ Python tracker

[issue26219] implement per-opcode cache in ceval

Changes by Yury Selivanov : -- hgrepos: +333 ___ Python tracker ___ ___

[issue26219] implement per-opcode cache in ceval

Brett Cannon added the comment: If you run hg.python.org/benchmarks on Linux it has a flag to measure memory. -- ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

STINNER Victor added the comment: If I understand correctly, this change requires to wait until the PEP 509 is accepted, right? Well, I'm not really suprised, since global cache is mentioned as an use case of the PEP 509, and other global cache patches are mentioned in Prior Art. --

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: Yes, this patch depends on PEP 509. -- ___ Python tracker ___ ___

[issue26219] implement per-opcode cache in ceval

Changes by Yury Selivanov : -- assignee: yselivanov components: Interpreter Core nosy: yselivanov priority: normal severity: normal stage: patch review status: open title: implement per-opcode cache in ceval type: performance versions: Python 3.6

[issue26219] implement per-opcode cache in ceval

New submission from Brett Cannon: I assume there's going to be a patch or more of a description of what your idea is? :) -- nosy: +brett.cannon ___ Python tracker

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: Yeah, I needed a URL of the issue for my email to python-dev ;) Here's a link to the email, that explains a lot about this patch: https://mail.python.org/pipermail/python-dev/2016-January/142945.html The patch is also attached (opcache1.patch). --

[issue26219] implement per-opcode cache in ceval

Changes by Yury Selivanov : -- nosy: +haypo, ncoghlan ___ Python tracker ___ ___

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: > If you run hg.python.org/benchmarks on Linux it has a flag to measure memory. Great. I'll re-run the benchmarks. BTW, the latest results are here: https://gist.github.com/1st1/aed69d63a2ff4de4c7be -- ___ Python

[issue26219] implement per-opcode cache in ceval

Changes by Yury Selivanov : -- dependencies: +Speedup method calls 1.2x ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

Changes by Yury Selivanov : -- dependencies: +PEP 509: Add ma_version to PyDictObject ___ Python tracker ___

[issue26219] implement per-opcode cache in ceval

Yury Selivanov added the comment: BTW, there are a couple of unit-tests that fail. Both can be easily fixed. To really move this thing forward, we need to profile the memory usage. First, it would be interesting to see how much additional memory is consumed if we optimize every code object.