-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
add internal asyncio implementation docs #135469
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be honest this looks more like a thorough description of and rationale for the changes to support free-threading (and asyncio pstree
), than a description of asyncio internals. As such, I think it belongs somewhere but I'm not sure if InternalDocs is the right place. Maybe @iritkatriel has an opinion?)
Nit: Call me old-fashioned, but could you please break lines longer than 100 or 120 characters? (Preferably at commas, semicolons, colons and periods.)
Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I now follow the reasoning. LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still would like to see lines broken at logical breakpoints to be at most 100 characters wide, but I won't insist on it. (I do note that all other docs in this file break lines longer than 80-100 characters.)
I agree that the information here is all useful to document, but also agree with @gvanrossum's comment about the structure of the doc. The purpose of the internals docs is to help contributors navigate the current (this version's) codebase. Rationale for changes should ideally be in the issues that describe the changes. I think the best approach here would be to document the pre-free-threading implementation, backport it, and then change it to describe the 3.14 implementation (perhaps with links to the issues that discuss the rationale). The description of the current implementation can include a brief list of the requirements of the implementation:
Then the reader can see that the implementation meets the requirements. But I don't think the current version's document needs to explain that previous implementations didn't meet the requirements. Also agree with wrapping long lines, as we do in other docs. |
The current internal docs like interpreter.md describe the current implementation as well as previous implementation which IMO makes it easier to see easily see what all changed so I wrote this doc similarly.
I had plans to document more stuff like cancellation semantics at some point, but keeping docs in separate branches would make it harder to determine what exactly changed and sounds like a lot more work to me. If we only want to keep the current versions docs in here then I think internal docs is the wrong place for this, maybe I should keep it somewhere else? |
It's not that "we only want to keep current versions in this doc", but that we want the focus of this doc to be the current version and not the delta between 3.13 and 3.14 (how will this document evolve when more changes are made in the future?) The purpose of the internal docs is to help contributors find their way in the codebase. Delta is described in the issues that propose changes.
I think you are referring to the sentence "Up through 3.10, the call stack was implemented as a singly-linked list of frame objects. This was expensive because each call would require a heap allocation for the stack frame." Is there any other reference to previous implementations in interpreter.md? That brief reference to a previous version doesn't make the delta between the versions the focus of the doc in the way that it's done in this PR. My opinion is only my opinion. @gvanrossum approved the PR, so you can merge it if you think this is a useful way to present the information to future contributors. |
This documents the work I did in 3.14 for better performance in both free-threading and gil builds and scaling asyncio in free-threading.