Skip to content

Migrate to a C extension for better performance#710

Closed
Vizonex wants to merge 4 commits intoaio-libs:masterfrom
Vizonex:C-Extension
Closed

Migrate to a C extension for better performance#710
Vizonex wants to merge 4 commits intoaio-libs:masterfrom
Vizonex:C-Extension

Conversation

@Vizonex
Copy link
Member

@Vizonex Vizonex commented Oct 22, 2025

WARNING

  • This PR IS STILL A DRAFT DO NOT MERGE YET. THIS IS NOT A FINAL DRAFT!!!
  • I still need to cleanup things and comments just fyi...
  • I still need to add a __reduce__ method for pickling

What do these changes do?

This is another concept of moving something on over to C that I mentioned doing in the aiolibs matrix server to see if something could be optimized a bit more. These changes also include better documentation by throwing
the functions documented in the readthedocs into the C Module and Pure Python Module.

I have already spent about 12 hours all by hand in one day making this from my own inspiration from other things I've seen over the past couple of years, if this pr is turned down feel free to take what you want from it so that my work or ideas are at least being used elsewhere.

My main objective with this PR was to act as a better bridge between FrozenList <-> list and removing all costly calls as well as better integration for atomic variables in C inspired by Python's new atomic variables.

Are there changes in behavior for the user?

  • Installation Might be different
  • Workflows will need changing
  • Major Version Number may be upped due to this large change to 2.X.X

Related issue number

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modifications, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep the list in alphabetical order, the file is sorted by name.
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."
  • (TODO) IF WE GO DOWN THIS ROUTE We need to double check refs and memory since we seem to be really good at screwing up refcounts before a big release takes place (examples of previous bug incidents: multidict & propcache)

Comment on lines +36 to +37
# def __mul__(self, value):
# return self.__class__(self._items.__mul__(value))

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
# def __mul__(self, value):
# return self.__class__(self._items.__mul__(value))

def __imul__(self, value):

Check notice

Code scanning / CodeQL

Non-standard exception raised in special method Note

This method raises
RuntimeError
- should raise an ArithmeticError or return NotImplemented instead.
Comment on lines +45 to +49
# def __rmul__(self, value):
# return self.__class__(self._items.__rmul__(value))

# def __add__(self, value):
# return self.__class__(self._items.__add__(value))

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
def __contains__(self, value):
return self._items.__contains__(value)

def __iadd__(self, values):

Check notice

Code scanning / CodeQL

Non-standard exception raised in special method Note

This method raises
RuntimeError
- should raise an ArithmeticError or return NotImplemented instead.
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

Normal methods should have 'self', rather than 'regs', as their first parameter.
# 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

Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.
encoding = "utf-8"
errors = "surrogateescape"

with open(filename, encoding=encoding, errors=errors) as fp:

Check notice

Code scanning / CodeQL

Empty except Note

'except' clause does nothing but pass and there is no explanatory comment.
@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.85%. Comparing base (eedf0a2) to head (467d1dd).
⚠️ Report is 14 commits behind head on master.

Files with missing lines Patch % Lines
frozenlist/__init__.py 66.66% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #710      +/-   ##
==========================================
+ Coverage   94.90%   98.85%   +3.95%     
==========================================
  Files          10        2       -8     
  Lines         726      350     -376     
  Branches       48       26      -22     
==========================================
- Hits          689      346     -343     
+ Misses         13        4       -9     
+ Partials       24        0      -24     
Flag Coverage Δ
CI-GHA 98.57% <66.66%> (+3.80%) ⬆️
MyPy ?
OS-Linux 98.57% <66.66%> (-1.14%) ⬇️
OS-Windows 98.57% <66.66%> (-1.14%) ⬇️
OS-macOS 98.57% <66.66%> (-1.14%) ⬇️
Py-3.10.11 98.57% <66.66%> (-1.14%) ⬇️
Py-3.10.18 98.57% <66.66%> (-1.14%) ⬇️
Py-3.11.13 98.57% <66.66%> (-1.14%) ⬇️
Py-3.11.9 98.57% <66.66%> (-1.14%) ⬇️
Py-3.12.10 98.57% <66.66%> (-1.14%) ⬇️
Py-3.12.11 98.57% <66.66%> (-1.14%) ⬇️
Py-3.13.5 98.57% <66.66%> (-1.14%) ⬇️
Py-3.13.7 98.57% <66.66%> (-1.14%) ⬇️
Py-3.13.8 98.57% <66.66%> (-1.14%) ⬇️
Py-3.13.9t 98.57% <66.66%> (-1.14%) ⬇️
Py-3.14.0 98.57% <66.66%> (-1.14%) ⬇️
Py-3.14.0t 98.57% <66.66%> (-1.14%) ⬇️
Py-3.9.13 98.57% <66.66%> (-1.14%) ⬇️
Py-3.9.23 98.57% <66.66%> (-1.14%) ⬇️
Py-pypy3.10.16-7.3.19 97.71% <66.66%> (-1.11%) ⬇️
Py-pypy3.9.19-7.3.16 97.71% <66.66%> (-1.11%) ⬇️
VM-macos-latest 98.57% <66.66%> (-1.14%) ⬇️
VM-ubuntu-latest 98.57% <66.66%> (-1.14%) ⬇️
VM-windows-11-arm 98.57% <66.66%> (-1.14%) ⬇️
VM-windows-latest 98.57% <66.66%> (-1.14%) ⬇️
pytest 98.57% <66.66%> (-1.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@webknjaz
Copy link
Member

Is the maintenance burden justified, though? Also, if we were to migrate to something more native, I'd go for Rust and not C.

@Vizonex
Copy link
Member Author

Vizonex commented Oct 22, 2025

Is the maintenance burden justified, though? Also, if we were to migrate to something more native, I'd go for Rust and not C.

I think I agree with you this time. I'm just a bit of a noob at PyO3 but maybe I'll look into trying it.

@webknjaz
Copy link
Member

Personally, I wouldn't want to maintain another thing in C. Especially since it's not as forward-compatible as Cython.

@Vizonex
Copy link
Member Author

Vizonex commented Oct 24, 2025

Personally, I wouldn't want to maintain another thing in C. Especially since it's not as forward-compatible as Cython.

Perhaps I could look into modifying the current cython code to have something simillar to what I have implemented here then.

@Vizonex
Copy link
Member Author

Vizonex commented Oct 24, 2025

@webknjaz Moved to #712

@Vizonex Vizonex closed this Oct 24, 2025
@Vizonex Vizonex deleted the C-Extension branch October 24, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments