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

pyc file with a large tuple is ~5x bigger in 3.11+ than in 3.9 #109036

Closed
fried opened this issue Sep 6, 2023 · 5 comments
Closed

pyc file with a large tuple is ~5x bigger in 3.11+ than in 3.9 #109036

fried opened this issue Sep 6, 2023 · 5 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes type-bug An unexpected behavior, bug, or error

Comments

@fried
Copy link
Contributor

fried commented Sep 6, 2023

Bug description:

The code to produce the regression is large see: https://gist.github.com/fried/75fc3e3477634927444693329444648c for the best example I could come up with that I could share.

Basically we have this old code generator for thrift that generates these things like DataClasses for use in serialization and rpc. Well there is a C level accelerator that helps to serialize and desalinize the structures so it generates a runtime available form of the rpc/field specs that we can pass into the accelerator. The form is an array where the array position is the field id in thrift. If a field id is skipped in the thrift specification you put a None in the empty positions in the array. This works well but there are some edge cases where structs skip large number of field ids, say going from field id 5 to field id 9000. You do this enough in a thrift file and you have many large tuples. To make a form of this I could share I just have one tuple with 20k entries.

Looking at the pyc sizes you can see there is a serious issue in 3.10

-rw-r--r-- 1 carljm users 6425726 Sep  6 16:08 bigpyc.cpython-310.pyc
-rw-r--r-- 1 carljm users  397049 Sep  6 16:14 bigpyc.cpython-311.pyc
-rw-r--r-- 1 carljm users  396930 Sep  6 16:12 bigpyc.cpython-312.pyc
-rw-r--r-- 1 carljm users  396930 Sep  6 16:08 bigpyc.cpython-313.pyc
-rw-r--r-- 1 carljm users   81114 Sep  6 16:19 bigpyc.cpython-38.pyc
-rw-r--r-- 1 carljm users   81114 Sep  6 16:15 bigpyc.cpython-39.pyc

This is directly translating to Memory usage of the imported pyc. I believe because in 3.10 the bytecode creates the tuples as a list then convert them to a tuple at import with LIST_TO_TUPLE. So we need the memory for a list and a tuple to exist at the same time so we see around a 2.8x memory regression in real world examples.

CPython versions tested on:

3.8, 3.9, 3.10, 3.11, 3.12, CPython main branch

Operating systems tested on:

Linux, macOS

@fried fried added the type-bug An unexpected behavior, bug, or error label Sep 6, 2023
@fried
Copy link
Contributor Author

fried commented Sep 6, 2023

@carljm for generating the list of pyc sizes for versions I didn't have installed.

@fried fried closed this as completed Sep 6, 2023
@fried fried reopened this Sep 6, 2023
@carljm
Copy link
Member

carljm commented Sep 6, 2023

Looks like one factor here is a compiler change from 3.9 to 3.10 that limits stack size used and thus switches away from a giant BUILD_TUPLE to a series of LIST_APPEND and then LIST_TO_TUPLE. But that doesn't fully explain the massive bytecode size in 3.10, or the remaining 5x regression in 3.11+ relative to 3.9. I suspect maybe the precise error locations, but haven't confirmed that yet.

@DinoV
Copy link
Contributor

DinoV commented Sep 7, 2023

It does look like co_lnotab is 6345620 on 3.10 and 40368 on 3.8.

@hugovk hugovk added 3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes labels Sep 7, 2023
@fried
Copy link
Contributor Author

fried commented Sep 7, 2023

I was able to mitigate this issue at Meta by making the generated tuple sparse in the .py and at import time expanding it with the None's so the c extensions are still happy.

@carljm carljm changed the title bytecode compilation regression involving tuples. Affects Memory and pyc size. pyc file with a large tuple is ~5x bigger in 3.11+ than in 3.9 Sep 7, 2023
facebook-github-bot pushed a commit to facebook/fbthrift that referenced this issue Sep 7, 2023
Summary:
In python 3.10 large structs are very expensive in bytecode compilation resulting in overly large pyc files and lots of memory regression.
see: python/cpython#109036

They are large because they are expanded to contain skipped thrift id's

To solve this we move to a sparse form of thrift_spec in the generated py source and expand at import time. This has shown to provide the following savings in skynet.

```
80% reduction in RSS for 3.10 and some win for 3.8
Generated Code is much smaller also

Before fix
py3.8 RSS = 161672,    Generated Code + pyc size 162M
py3.10 RSS = 681720,  Generated Code + pyc size 461M

After Fix
py3.8 RSS = 146700,  Generated Code + pyc size 90M
py3.10 RSS = 135260, Generated Code + pyc size 80M
```

Reviewed By: junzh0u

Differential Revision: D49041314

fbshipit-source-id: 2e3d84650a4b463a9431eea3cada12e6e0cb7037
facebook-github-bot pushed a commit to facebook/hhvm that referenced this issue Sep 7, 2023
Summary:
In python 3.10 large structs are very expensive in bytecode compilation resulting in overly large pyc files and lots of memory regression.
see: python/cpython#109036

They are large because they are expanded to contain skipped thrift id's

To solve this we move to a sparse form of thrift_spec in the generated py source and expand at import time. This has shown to provide the following savings in skynet.

```
80% reduction in RSS for 3.10 and some win for 3.8
Generated Code is much smaller also

Before fix
py3.8 RSS = 161672,    Generated Code + pyc size 162M
py3.10 RSS = 681720,  Generated Code + pyc size 461M

After Fix
py3.8 RSS = 146700,  Generated Code + pyc size 90M
py3.10 RSS = 135260, Generated Code + pyc size 80M
```

Reviewed By: junzh0u

Differential Revision: D49041314

fbshipit-source-id: 2e3d84650a4b463a9431eea3cada12e6e0cb7037
@carljm
Copy link
Member

carljm commented Nov 20, 2023

Since the massive regression in 3.10 is already fixed, and the remaining regression here in 3.11+ looks to be explained by precise error locations (which were accepted knowing they would increase pyc file size), and there's no specific proposal here to further reduce pyc file sizes while maintaining functionality, I'm closing this issue.

@carljm carljm closed this as not planned Won't fix, can't repro, duplicate, stale Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants