Skip to content
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

bpo-46841: Quicken code in-place #31888

Merged
merged 37 commits into from
Mar 21, 2022

Conversation

brandtbucher
Copy link
Member

@brandtbucher brandtbucher commented Mar 15, 2022

This moves the bytecode to the end of the corresponding PyCodeObject, and quickens it in-place.

Related changes:

  • PyCodeObject is now more compact. I've removed the almost-always-unused co_varnames, co_freevars, and co_cellvars member caches, and rearranged some int members to fill some holes in the struct on 64-bit builds. co_code has been removed and replaced with _PyCode_GetCode, and co_quickened and co_firstinstr have been replaced with _PyCode_CODE.
  • _PyOpcode_Deopt is a new mapping from all opcodes to their un-quickened forms.
  • _PyOpcode_InlineCacheEntries is renamed to _PyOpcode_Caches, _Py_IncrementCountAndMaybeQuicken is renamed to _PyCode_Warmup, _Py_Quicken is renamed to _PyCode_Quicken, and _co_quickened is renamed to _co_code_adaptive (and is now a read-only memoryview).
  • We don't emit unused nonzero opargs anymore in the compiler.

https://bugs.python.org/issue46841

@brandtbucher
Copy link
Member Author

1% perf improvement too:

Slower (11):
- pickle_dict: 27.6 us +- 0.2 us -> 28.4 us +- 0.3 us: 1.03x slower
- html5lib: 65.3 ms +- 2.7 ms -> 67.2 ms +- 2.8 ms: 1.03x slower
- pickle_list: 4.33 us +- 0.05 us -> 4.46 us +- 0.05 us: 1.03x slower
- regex_v8: 23.1 ms +- 0.2 ms -> 23.8 ms +- 0.2 ms: 1.03x slower
- regex_dna: 217 ms +- 1 ms -> 223 ms +- 4 ms: 1.03x slower
- scimark_lu: 111 ms +- 2 ms -> 113 ms +- 2 ms: 1.02x slower
- regex_effbot: 3.46 ms +- 0.06 ms -> 3.50 ms +- 0.05 ms: 1.01x slower
- json_dumps: 12.6 ms +- 0.1 ms -> 12.8 ms +- 0.2 ms: 1.01x slower
- fannkuch: 397 ms +- 3 ms -> 400 ms +- 5 ms: 1.01x slower
- json_loads: 28.1 us +- 0.3 us -> 28.3 us +- 0.3 us: 1.01x slower
- xml_etree_iterparse: 105 ms +- 1 ms -> 105 ms +- 1 ms: 1.01x slower

Faster (43):
- go: 149 ms +- 1 ms -> 139 ms +- 1 ms: 1.07x faster
- logging_simple: 5.38 us +- 0.10 us -> 5.16 us +- 0.08 us: 1.04x faster
- pickle: 9.89 us +- 0.14 us -> 9.53 us +- 0.10 us: 1.04x faster
- pycparser: 1.24 sec +- 0.02 sec -> 1.19 sec +- 0.02 sec: 1.04x faster
- thrift: 780 us +- 13 us -> 754 us +- 8 us: 1.03x faster
- deltablue: 3.85 ms +- 0.05 ms -> 3.73 ms +- 0.05 ms: 1.03x faster
- unpack_sequence: 48.5 ns +- 0.5 ns -> 47.1 ns +- 0.9 ns: 1.03x faster
- scimark_sparse_mat_mult: 4.97 ms +- 0.15 ms -> 4.83 ms +- 0.11 ms: 1.03x faster
- pyflate: 451 ms +- 3 ms -> 438 ms +- 4 ms: 1.03x faster
- xml_etree_process: 56.7 ms +- 0.8 ms -> 55.2 ms +- 0.7 ms: 1.03x faster
- pickle_pure_python: 323 us +- 3 us -> 314 us +- 3 us: 1.03x faster
- telco: 6.80 ms +- 0.09 ms -> 6.65 ms +- 0.16 ms: 1.02x faster
- scimark_sor: 120 ms +- 1 ms -> 118 ms +- 1 ms: 1.02x faster
- pidigits: 194 ms +- 0 ms -> 190 ms +- 0 ms: 1.02x faster
- logging_format: 5.87 us +- 0.08 us -> 5.74 us +- 0.09 us: 1.02x faster
- unpickle_pure_python: 238 us +- 2 us -> 233 us +- 2 us: 1.02x faster
- xml_etree_generate: 80.0 ms +- 0.6 ms -> 78.4 ms +- 0.7 ms: 1.02x faster
- meteor_contest: 108 ms +- 3 ms -> 106 ms +- 1 ms: 1.02x faster
- regex_compile: 139 ms +- 1 ms -> 136 ms +- 1 ms: 1.02x faster
- hexiom: 6.96 ms +- 0.03 ms -> 6.83 ms +- 0.02 ms: 1.02x faster
- sympy_sum: 163 ms +- 2 ms -> 160 ms +- 1 ms: 1.02x faster
- tornado_http: 98.2 ms +- 1.3 ms -> 96.5 ms +- 1.4 ms: 1.02x faster
- dulwich_log: 65.8 ms +- 0.4 ms -> 64.7 ms +- 0.5 ms: 1.02x faster
- sympy_integrate: 20.9 ms +- 0.1 ms -> 20.6 ms +- 0.1 ms: 1.02x faster
- scimark_fft: 340 ms +- 4 ms -> 334 ms +- 4 ms: 1.02x faster
- 2to3: 267 ms +- 1 ms -> 263 ms +- 1 ms: 1.02x faster
- scimark_monte_carlo: 69.7 ms +- 1.2 ms -> 68.7 ms +- 0.8 ms: 1.01x faster
- django_template: 35.0 ms +- 0.5 ms -> 34.5 ms +- 0.5 ms: 1.01x faster
- chaos: 71.7 ms +- 0.6 ms -> 70.7 ms +- 0.6 ms: 1.01x faster
- nbody: 94.0 ms +- 1.7 ms -> 92.8 ms +- 1.8 ms: 1.01x faster
- raytrace: 310 ms +- 2 ms -> 306 ms +- 3 ms: 1.01x faster
- sqlalchemy_declarative: 141 ms +- 3 ms -> 140 ms +- 3 ms: 1.01x faster
- float: 76.7 ms +- 0.8 ms -> 75.8 ms +- 1.0 ms: 1.01x faster
- sympy_str: 291 ms +- 2 ms -> 287 ms +- 3 ms: 1.01x faster
- richards: 47.5 ms +- 1.2 ms -> 47.0 ms +- 1.1 ms: 1.01x faster
- sympy_expand: 485 ms +- 6 ms -> 480 ms +- 3 ms: 1.01x faster
- python_startup_no_site: 6.02 ms +- 0.00 ms -> 5.96 ms +- 0.00 ms: 1.01x faster
- chameleon: 6.63 ms +- 0.07 ms -> 6.57 ms +- 0.06 ms: 1.01x faster
- crypto_pyaes: 83.9 ms +- 0.7 ms -> 83.2 ms +- 1.1 ms: 1.01x faster
- spectral_norm: 102 ms +- 1 ms -> 101 ms +- 1 ms: 1.01x faster
- python_startup: 8.41 ms +- 0.01 ms -> 8.34 ms +- 0.01 ms: 1.01x faster
- nqueens: 86.1 ms +- 1.2 ms -> 85.5 ms +- 0.8 ms: 1.01x faster
- pathlib: 18.3 ms +- 0.2 ms -> 18.2 ms +- 0.3 ms: 1.01x faster

Benchmark hidden because not significant (8): json, logging_silent, mako, sqlalchemy_imperative, sqlite_synth, unpickle, unpickle_list, xml_etree_parse

Geometric mean: 1.01x faster

@markshannon
Copy link
Member

This is still marked as draft, what is left to do?

@brandtbucher
Copy link
Member Author

This is still marked as draft, what is left to do?

There is still an awkward spot in _gen_throw where we walk back f_lasti to the previous SEND instruction and perform the jump ourselves when in a yield from (which is sort of a strange control-flow path that isn’t reflected in the CFG/bytecode/dis). I also don’t think the current implementation handles EXTENDED_ARGs correctly.

I’m still trying to understand the code better and figure out a cleaner way of doing this. Any ideas?

@brandtbucher
Copy link
Member Author

There is still an awkward spot in _gen_throw where we walk back f_lasti to the previous SEND instruction and perform the jump ourselves when in a yield from (which is sort of a strange control-flow path that isn’t reflected in the CFG/bytecode/dis). I also don’t think the current implementation handles EXTENDED_ARGs correctly.

I’m still trying to understand the code better and figure out a cleaner way of doing this. Any ideas?

#31968 should help.

@markshannon
Copy link
Member

I think it is OK to sort out the peculiarities of gen.throw() after this PR is merged.

@brandtbucher brandtbucher marked this pull request as ready for review March 18, 2022 20:08
@brandtbucher brandtbucher requested review from tiran and 1st1 as code owners March 18, 2022 20:08
@brandtbucher brandtbucher added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section and removed DO-NOT-MERGE labels Mar 18, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @brandtbucher for commit c8054b9 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 18, 2022
@markshannon
Copy link
Member

Buildbot failures are all pre-existing failures and unrelated to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants