[3.11] gh-93382: Cache result of `PyCode_GetCode` in codeobject (GH-93383) by Fidget-Spinner · Pull Request #93493 · python/cpython · GitHub

(cherry picked from commit d52ffc1)

PyCode_GetCode could be faster #93382

PyCode_GetCode could be faster #93382

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

@pablogsal and @nedbat

This PR improves coverage performance on 3.11 by 5-7%. Using Ned's benchmarks

# 3.11 branch at 1497d7fdefff8207b8ccde82e96b6f471416c284
Median for bm_sudoku.py, python3.11, cov=none: 10.476s
Median for bm_sudoku.py, python3.11, cov=6.4.1: 69.124s
Median for bm_spectral_norm.py, python3.11, cov=none: 9.072s
Median for bm_spectral_norm.py, python3.11, cov=6.4.1: 72.143s

# 3.11 co_code_cached
Median for bm_sudoku.py, python3.11, cov=none: 10.363s
Median for bm_sudoku.py, python3.11, cov=6.4.1: 64.726s
Median for bm_spectral_norm.py, python3.11, cov=none: 9.325s
Median for bm_spectral_norm.py, python3.11, cov=6.4.1: 69.713s

An observation: bm_spectral_norm improved less than bm_sudoku because the size of its co_code is smaller. So the cost of getting a new one every time isn't as high. I think real-world code sizes are more likely to be like bm_sudoku or larger. So I'd guestimate we will see ~10% improvement in real-world code running coverage in 3.11.

Unfortunately this means that whatever is making Python 3.11 slower with coverage, unfortunately is not only this :(

Great investigation @Fidget-Spinner and thanks for working on this ♥️

I will try to review ASAP but it would be great if @markshannon @iritkatriel @brandtbucher or @ericsnowcurrently can take a look.

I'm on mobile right now (probably can't get to a computer today), but here are a few thoughts based on a first look:

I also ran the benchmarks using #93493:

cov	proj	python3.10	python3.11	gh93493	3.11 vs 3.10	gh93493 vs 3.10
none	bug1339.py	0.193 s	0.155 s	0.143 s	0.803	0.743
none	bm_sudoku.py	10.686 s	10.393 s	11.867 s	0.973	1.111
none	bm_spectral_norm.py	16.051 s	10.940 s	10.987 s	0.682	0.684
6.4.1	bug1339.py	0.439 s	0.842 s	0.771 s	1.918	1.757
6.4.1	bm_sudoku.py	30.148 s	61.392 s	61.606 s	2.036	2.043
6.4.1	bm_spectral_norm.py	40.672 s	79.562 s	73.221 s	1.956	1.800

It's a slight improvement, but isn't solving the problem.

It's a slight improvement, but isn't solving the problem.

Yeah I'm aware. I sent this PR in because 10% improvement on macrobenchmarks is still something! Even cProfile slowed down by 60% when profiling code in 3.11. My hunch is that accessing the full PyFrameObject is signifcantly more expensive now in 3.11. However, I can't fix that because it's part of the tracing Py_tracefunc C API.

I also ran the benchmarks using #93493:

It's a slight improvement, but isn't solving the problem.

Hm, it looks like in some cases this actually makes things a bit slower (perhaps due to the extra memory consumption)?

Maybe we should pause this PR until @markshannon has finished reworking the line number calculations, which seem to be the bulk of the issue at this point. Or at least see if it slows down pyperformance at all before merging?

I also ran the benchmarks using #93493:

It's a slight improvement, but isn't solving the problem.

Hm, it looks like in some cases this actually makes things a bit slower (perhaps due to the extra memory consumption)?

Maybe we should pause this PR until @markshannon has finished reworking the line number calculations, which seem to be the bulk of the issue at this point. Or at least see if it slows down pyperformance at all before merging?

When I benchmarked on the main/3.12 branch, it didn't make anything slower #93383 (comment). However, I agree on waiting for a while.

When benchmarking with Ned's benchmarks, I saw a slowdown in bm_sudoku but no slowdown in bm_spectral_norm. I'm not sure what's up with that.

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Co-Authored-By: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

…port

I have made the requested changes; please review again Mark.

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.

Looks good.

LGTM

Fidget-Spinner and others added 2 commits June 4, 2022 20:13

pythongh-93382 : Cache result of PyCode_GetCode in codeobject (pytho…

13642ce

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

Update test_frozenmain.h

0f9511f

Fidget-Spinner requested a review from markshannon as a code owner June 4, 2022 12:16

bedevere-bot added the awaiting core review label Jun 4, 2022

Fidget-Spinner changed the title [3.11] Cache result of PyCode_GetCode in codeobject (GH-93383) [3.11] gh-93382: Cache result of PyCode_GetCode in codeobject (GH-93383) Jun 4, 2022

Fidget-Spinner mentioned this pull request Jun 4, 2022

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

Merged

Fidget-Spinner requested a review from brandtbucher June 4, 2022 12:39

brandtbucher reviewed Jun 4, 2022

View reviewed changes

Comment thread Include/cpython/code.h Outdated

Comment thread Objects/codeobject.c

Address Brandt's review

56b017d

brandtbucher reviewed Jun 8, 2022

View reviewed changes

Comment thread Programs/test_frozenmain.h

markshannon requested changes Jun 10, 2022

View reviewed changes

Comment thread Include/cpython/code.h Outdated

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

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

bedevere-bot added the awaiting changes label Jun 10, 2022

Apply suggestions by Mark

5af4e76

kumaraditya303 mentioned this pull request Jun 11, 2022

Memory leak when reading co_code attribute of deepfrozen code objects #93728

Closed

Fix memory leak in deepfrozen code objects

be1baad

Co-Authored-By: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

nedbat mentioned this pull request Jun 12, 2022

Severe performance degradation for tracing under 3.11 #93516

Open

Fidget-Spinner added 3 commits June 22, 2022 23:41

Merge remote-tracking branch 'upstream/3.11' into co_code_cached_back…

c979855

…port

add to deepfreeze.py

887120f

make regen-abidump

b34659c

bedevere-bot added awaiting change review and removed awaiting changes labels Jun 23, 2022

bedevere-bot requested a review from markshannon June 23, 2022 12:11

markshannon approved these changes Jun 23, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Jun 23, 2022

Fidget-Spinner requested a review from pablogsal June 23, 2022 12:41

kumaraditya303 approved these changes Jun 23, 2022

View reviewed changes

pablogsal merged commit 852b4d4 into python:3.11 Jun 23, 2022

bedevere-bot removed the awaiting merge label Jun 23, 2022

Fidget-Spinner mentioned this pull request Jun 24, 2022

gh-93382: Sync up co_code changes with 3.11 #94227

Merged

Fidget-Spinner deleted the co_code_cached_backport branch June 24, 2022 17:54

[3.11] gh-93382: Cache result of `PyCode_GetCode` in codeobject (GH-93383) by Fidget-Spinner · Pull Request #93493 · python/cpython · GitHub

Uh oh!

Uh oh!

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

[3.11] gh-93382: Cache result of PyCode_GetCode in codeobject (GH-93383)#93493pablogsal merged 8 commits intopython:3.11python/cpython:3.11from Fidget-Spinner:co_code_cached_backportFidget-Spinner/cpython:co_code_cached_backportCopy head branch name to clipboard

Conversation

Fidget-Spinner commented Jun 4, 2022 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fidget-Spinner commented Jun 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pablogsal commented Jun 4, 2022

Uh oh!

brandtbucher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nedbat commented Jun 5, 2022

Uh oh!

Fidget-Spinner commented Jun 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

brandtbucher commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fidget-Spinner commented Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Jun 10, 2022

Uh oh!

Fidget-Spinner commented Jun 23, 2022

Uh oh!

bedevere-bot commented Jun 23, 2022

Uh oh!

markshannon left a comment

Choose a reason for hiding this comment

Uh oh!

kumaraditya303 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

[3.11] gh-93382: Cache result of PyCode_GetCode in codeobject (GH-93383)#93493
pablogsal merged 8 commits into
python:3.11from
Fidget-Spinner:co_code_cached_backport

Fidget-Spinner commented Jun 4, 2022 •
edited by bedevere-bot
Loading

Fidget-Spinner commented Jun 4, 2022 •
edited
Loading

Fidget-Spinner commented Jun 5, 2022 •
edited
Loading

brandtbucher commented Jun 8, 2022 •
edited
Loading

Fidget-Spinner commented Jun 9, 2022 •
edited
Loading