gh-93382: Cache result of `PyCode_GetCode` in codeobject #93383

Fidget-Spinner · 2022-05-31T16:34:54Z

Fidget-Spinner · 2022-05-31T16:39:25Z

@markshannon do tell me if you feel that it isn't worth making PyCodeObject larger for this use case. I think it's worth it because it restores O(1) co_code access. Currently it's O(kn).

Benchmarks aren't available yet. I'll try to run it by the end of this week if no one beats me to it.

Fidget-Spinner · 2022-06-01T03:19:59Z

Quick n dirty microbenchmark:

py -3.10 -m timeit -s "import typing; c = typing.get_type_hints.__code__" "c.co_code"
10000000 loops, best of 5: 27.4 nsec per loop

py -3.11 -m timeit -s "import typing; c = typing.get_type_hints.__code__" "c.co_code"
500000 loops, best of 5: 555 nsec per loop

# This branch, release build without PGO
 .\PCbuild\amd64\python.exe -m timeit -s "import typing; c = typing.get_type_hints.__code__" "c.co_code"
10000000 loops, best of 5: 28.6 nsec per loop

Fidget-Spinner · 2022-06-03T08:33:16Z

No real change in pyperformance https://gist.github.com/Fidget-Spinner/55cd7ee60faaa403ddc970ec31401d35. Seems like weird build differences/noise only.

Misc/NEWS.d/next/Core and Builtins/2022-05-31-16-36-30.gh-issue-93382.Jf6gAj.rst

…e-93382.Jf6gAj.rst Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

Fidget-Spinner · 2022-06-03T09:56:45Z

Thanks for the review Kumar!

Fidget-Spinner · 2022-06-03T13:50:36Z

I'm completely confused. How are tests relating to the peepholer now failing after we updated the news entry...?

…o_code

sweeneyde · 2022-06-03T14:44:04Z

Without looking at it too carefully yet, I'm guessing the cache needs to be invalidated when the bytecode changes here.

Co-Authored-By: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>

Fidget-Spinner · 2022-06-03T16:11:10Z

Without looking at it too carefully yet, I'm guessing the cache needs to be invalidated when the bytecode changes here.

I think you're spot on! Thank you.

Fidget-Spinner · 2022-06-03T16:40:55Z

Since there are no perf regressions, I'm merging this.

Fidget-Spinner · 2022-06-03T16:44:59Z

@pablogsal can I backport this PR to 3.11 please?

In essence, 3.11 beta 1 made accessing co_code O(kn) where it was previously O(1) in prior versions. I want to fix that performance regression because it affects anything that touches co_code (e.g coverage.py).

See e.g #93383 (comment) for microbenchmark. For even just a medium-sized code object, there's a nearly 20x slowdown from 3.10 to 3.11. I can't imagine how bad it would be for larger code in the wild.

This change affects the code object struct. I'm not sure if that will affect your decision.

Fidget-Spinner · 2022-06-04T03:20:21Z

@kumaraditya303 I have one question about test_frozenmain and I think you have more expertise in this area than I do:

Seems like test_frozenmain was regenerated for this PR. Does this mean or indicate anything? I'm afraid that I inadvertently changed bytecode representation.

kumaraditya303 · 2022-06-04T06:38:20Z

Marshal format is not deterministic and it seems to be a form of #73894 issue.

Here is the diffoscope diff:

@@ -1,10 +1,10 @@
 00000000: ae0d 0d0a 0000 0000 4c71 5d62 bc01 0000  ........Lq]b....
 00000010: e300 0000 0000 0000 0000 0000 0008 0000  ................
-00000020: 0000 0000 0073 a000 0000 9700 6400 6401  .....s......d.d.
+00000020: 0000 0000 00f3 a000 0000 9700 6400 6401  ............d.d.
 00000030: 6c00 5a00 6400 6401 6c01 5a01 0200 6502  l.Z.d.d.l.Z...e.
 00000040: 6402 ab01 0000 0000 0000 0000 0100 0200  d...............
 00000050: 6502 6403 6500 6a03 0000 0000 0000 0000  e.d.e.j.........
 00000060: ab02 0000 0000 0000 0000 0100 0200 6501  ..............e.
 00000070: 6a04 0000 0000 0000 0000 ab00 0000 0000  j...............
 00000080: 0000 0000 6404 1900 0000 0000 0000 0000  ....d...........
 00000090: 5a05 6405 4400 5d17 5a06 0200 6502 6406  Z.d.D.].Z...e.d.
@@ -18,21 +18,21 @@
 00000110: 6162 6c65 da0f 7573 655f 656e 7669 726f  able..use_enviro
 00000120: 6e6d 656e 74da 1163 6f6e 6669 6775 7265  nment..configure
 00000130: 5f63 5f73 7464 696f da0e 6275 6666 6572  _c_stdio..buffer
 00000140: 6564 5f73 7464 696f 7a07 636f 6e66 6967  ed_stdioz.config
 00000150: 207a 023a 2029 07da 0373 7973 da11 5f74   z.: )...sys.._t
 00000160: 6573 7469 6e74 6572 6e61 6c63 6170 69da  estinternalcapi.
 00000170: 0570 7269 6e74 da04 6172 6776 da0b 6765  .print..argv..ge
-00000180: 745f 636f 6e66 6967 7372 0200 0000 da03  t_configsr......
+00000180: 745f 636f 6e66 6967 7372 0300 0000 da03  t_configsr......
 00000190: 6b65 79a9 00f3 0000 0000 fa1b 5072 6f67  key.........Prog
 000001a0: 7261 6d73 2f74 6573 745f 6672 6f7a 656e  rams/test_frozen
 000001b0: 6d61 696e 2e70 79fa 083c 6d6f 6475 6c65  main.py..<module
-000001c0: 3e72 1100 0000 0100 0000 738c 0000 00f8  >r........s.....
+000001c0: 3e72 1200 0000 0100 0000 738c 0000 00f8  >r........s.....
 000001d0: f006 0001 0b80 0a80 0a80 0ad8 0018 d000  ................
 000001e0: 18d0 0018 d000 18e0 0005 8005 d006 1ad4  ................
 000001f0: 001b d000 1bd8 0005 8005 806a 9023 9428  ...........j.#.(
 00000200: d400 1bd0 001b d809 26d0 091a d409 26d4  ........&.....&.
 00000210: 0928 a818 d409 3280 06f0 0206 0c02 f000  .(....2.........
 00000220: 0701 2af0 0007 012a 8043 f00e 0005 0a80  ..*....*.C......
 00000230: 45d0 0a28 9043 d00a 28d0 0a28 9836 a023  E..(.C..(..(.6.#
 00000240: 9c3b d00a 28d0 0a28 d404 29d0 0429 d004  .;..(..(..)..)..
-00000250: 29f0 0f07 012a f000 0701 2a72 0f00 0000  )....*....*r....
+00000250: 29f0 0f07 012a f000 0701 2a72 1000 0000  )....*....*r....

cc @methane

kumaraditya303 · 2022-06-04T06:56:11Z

I'm afraid that I inadvertently changed bytecode representation.

No, I don't think so because I tried comparing deepfreeze output of test_frozenmain and it did not show any difference.

pablogsal · 2022-06-04T11:52:40Z

@pablogsal can I backport this PR to 3.11 please?

In essence, 3.11 beta 1 made accessing co_code O(kn) where it was previously O(1) in prior versions. I want to fix that performance regression because it affects anything that touches co_code (e.g coverage.py).

See e.g #93383 (comment) for microbenchmark. For even just a medium-sized code object, there's a nearly 20x slowdown from 3.10 to 3.11. I can't imagine how bad it would be for larger code in the wild.

This change affects the code object struct. I'm not sure if that will affect your decision.

Can you please check if this fixes this issue:

nedbat/coveragepy#1287

If so, I am happy backporting it as long as two more core dev other than myself review it and approve it.

CC: @markshannon @brandtbucher @nedbat

Fidget-Spinner · 2022-06-04T12:09:27Z

Can you please check if this fixes this issue:

It should help but I doubt it will fix all of it. My own research suggests that profiling has slowed down by >50% in 3.11, even without the co_code issue.

…nGH-93383) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>

Fidget-Spinner · 2022-06-04T12:17:39Z

I opened the backport for 3.11 since it's simpler than the 3.12 code. I hope the discussion can happen there or on the issue instead #93493. Thank you!

…3383) (#93493) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Co-authored-by: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>

Cache result of PyCode_GetCode in codeobject

ed061f6

Fidget-Spinner requested a review from markshannon as a code owner May 31, 2022 16:34

bedevere-bot added the awaiting core review label May 31, 2022

📜🤖 Added by blurb_it.

1077bd3

AA-Turner added performance Performance or resource usage interpreter-core (Objects, Python, Grammar, and Parser dirs) labels May 31, 2022

Fidget-Spinner requested a review from sweeneyde June 3, 2022 08:33

kumaraditya303 reviewed Jun 3, 2022

View reviewed changes

Misc/NEWS.d/next/Core and Builtins/2022-05-31-16-36-30.gh-issue-93382.Jf6gAj.rst Outdated Show resolved Hide resolved

kumaraditya303 approved these changes Jun 3, 2022

View reviewed changes

Update Misc/NEWS.d/next/Core and Builtins/2022-05-31-16-36-30.gh-issu…

76ada15

…e-93382.Jf6gAj.rst Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

Merge branch 'main' of https://github.com/python/cpython into cache_c…

f407dab

…o_code

Fidget-Spinner and others added 3 commits June 3, 2022 23:07

invalidate cached code object when frame adds LOAD_FAST_CHECK

fde8a30

Co-Authored-By: Dennis Sweeney <36520290+sweeneyde@users.noreply.github.com>

Py_CLEAR already checks for NULL

0c7226c

only invalidate cache if opcode changed

0b11670

Fidget-Spinner merged commit d52ffc1 into python:main Jun 3, 2022

bedevere-bot removed the awaiting core review label Jun 3, 2022

Fidget-Spinner deleted the cache_co_code branch June 3, 2022 16:41

This was referenced Jun 8, 2022

Severe performance degradation for tracing under 3.11 #93516

Open

[3.11] gh-93382: Cache result of PyCode_GetCode in codeobject (GH-93383) #93493

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-93382: Cache result of `PyCode_GetCode` in codeobject #93383

gh-93382: Cache result of `PyCode_GetCode` in codeobject #93383

Fidget-Spinner commented May 31, 2022 •

edited

Loading

Fidget-Spinner commented May 31, 2022 •

edited

Loading

Fidget-Spinner commented Jun 1, 2022 •

edited

Loading

Fidget-Spinner commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022

sweeneyde commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022 •

edited

Loading

Fidget-Spinner commented Jun 4, 2022

kumaraditya303 commented Jun 4, 2022

kumaraditya303 commented Jun 4, 2022

pablogsal commented Jun 4, 2022

Fidget-Spinner commented Jun 4, 2022 •

edited

Loading

Fidget-Spinner commented Jun 4, 2022 •

edited

Loading

gh-93382: Cache result of PyCode_GetCode in codeobject #93383

gh-93382: Cache result of PyCode_GetCode in codeobject #93383

Conversation

Fidget-Spinner commented May 31, 2022 • edited Loading

Fidget-Spinner commented May 31, 2022 • edited Loading

Fidget-Spinner commented Jun 1, 2022 • edited Loading

Fidget-Spinner commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022

sweeneyde commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022

Fidget-Spinner commented Jun 3, 2022 • edited Loading

Fidget-Spinner commented Jun 4, 2022

kumaraditya303 commented Jun 4, 2022

kumaraditya303 commented Jun 4, 2022

pablogsal commented Jun 4, 2022

Fidget-Spinner commented Jun 4, 2022 • edited Loading

Fidget-Spinner commented Jun 4, 2022 • edited Loading

gh-93382: Cache result of `PyCode_GetCode` in codeobject #93383

gh-93382: Cache result of `PyCode_GetCode` in codeobject #93383

Fidget-Spinner commented May 31, 2022 •

edited

Loading

Fidget-Spinner commented May 31, 2022 •

edited

Loading

Fidget-Spinner commented Jun 1, 2022 •

edited

Loading

Fidget-Spinner commented Jun 3, 2022 •

edited

Loading

Fidget-Spinner commented Jun 4, 2022 •

edited

Loading

Fidget-Spinner commented Jun 4, 2022 •

edited

Loading