Rough Draft Idea For Migrating Propcache to CPython #125
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
for more information, see https://pre-commit.ci
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| class Py_Is(Operation): | ||
| NAME = "Py_Is" | ||
|
|
||
| def replace2(regs): |
Check notice
Code scanning / CodeQL
First parameter of a method is not named 'self' Note
| # FIXME: add macro after the first header comment | ||
| # FIXME: add macro after includes | ||
| # FIXME: add macro after: #define PY_SSIZE_T_CLEAN | ||
| return line + "\n" + content |
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns Note
| encoding = "utf-8" | ||
| errors = "surrogateescape" | ||
|
|
||
| with open(filename, encoding=encoding, errors=errors) as fp: |
Check notice
Code scanning / CodeQL
Empty except Note
|
Personally, I don't think we should be writing anything new in C. We should be using Cython or Rust. But, I'll leave this to bdraco to decide where the project should go. |
|
I think we should only consider C if we can see a 50% or better speed up |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What do these changes do?
I moved Propcache to C out of pure boredom/inspiration from having worked a bit on the multidict project for a time so I felt a little bit confident that I could at least pull this off as an outlined plan for optimizing propcache's properties a bit further.
This however is a rough-draft/blue-print idea and like all should be treated as one. I do not expect to do anything with it for the time being and you should treat it as experimental until I am ready to actually begin merging it to it and start replacing the cython script in favor of it.
We will need to add clang-format to pre-commit before we wish to proceed with this large change and I don't expect for this to be implemented until after 0.3.3 has been rolled out.
Edit: Please know that I am a little bit unskilled with writing CPython all by myself so this rough draft might have a few hiccups in the C Code.
Are there changes in behavior for the user?
Should be a little bit more performance now that
NULLcould be utilized a bit elsewhere. I doubt anyone is going to want a C-API Capsule like I and avetlov are doing to multidict so there may not be a need for one (unless cython starts supporting stuff like it which would be an extremely rare case scenario, incase I made it so that true means success and false indicates failure in a few places in the main header filepc.h).Related issue number
Checklist