Skip to content

Skip unwanted checks in FrozenList.append Cythonized extension by using PyList_Append Directly. - #817

Open
Vizonex wants to merge 5 commits into
aio-libs:masterfrom
Vizonex:smaller-optimizations
Open

Skip unwanted checks in FrozenList.append Cythonized extension by using PyList_Append Directly.#817
Vizonex wants to merge 5 commits into
aio-libs:masterfrom
Vizonex:smaller-optimizations

Conversation

@Vizonex

@Vizonex Vizonex commented Jul 24, 2026

Copy link
Copy Markdown
Member

What do these changes do?

This is an improved version of #712 That takes this behavior in steps rather than all at once. I'm mainly concerned with getting rid of all of these snippets that cython generates as I find it annoying and unnecessary.

  // Up here is the code I'm trying to get rid of to bring FrozenList to it's maximum potential.
  if (unlikely(__pyx_v_self->_items == Py_None)) {
    PyErr_Format(PyExc_AttributeError, "'NoneType' object has no attribute '%.30s'", "append");
    __PYX_ERR(0, 112, __pyx_L1_error)
  }
  __pyx_t_2 = __Pyx_PyList_Append(__pyx_v_self->_items, __pyx_v_item); if (unlikely(__pyx_t_2 == ((int)-1))) __PYX_ERR(0, 112, __pyx_L1_error)

If we change the Cython code to call PyList_Append(self._items, item) instead the code is much more pleasing to cython and as these unwanted checks are skipped entirely.

__pyx_t_1 = __pyx_v_self->_items;
  __Pyx_INCREF(__pyx_t_1);
  __pyx_t_2 = PyList_Append(__pyx_t_1, __pyx_v_item); if (unlikely(__pyx_t_2 == ((int)-1))) __PYX_ERR(0, 112, __pyx_L1_error)
  __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;

while I do find keeping the code being another issue entirely and what may have been preventing #712 from passing. I came up with a better solution that instead does everything in simple steps making things easier to review and easier to merge.

Final Resort if this is not merged

If this PR fails my final resort is to go to the Cython mainainters for help with getting this all fixed.

Are there changes in behavior for the user?

This will not change anything for the end user in fact this will improve performance for the end user.

Related issue number

This closes #712 and I will plan a different route to get everything in to remove what I have demonstrated above.

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>.<category> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure category is one of the following:
      • .bugfix: Signifying a bug fix.
      • .feature: Signifying a new feature.
      • .breaking: Signifying a breaking change or removal of something public.
      • .doc: Signifying a documentation improvement.
      • .packaging: Signifying a packaging or tooling change that may be relevant to downstreams.
      • .contrib: Signifying an improvement to the contributor/development experience.
      • .misc: Anything that does not fit the above; usually, something 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."

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9538c5b) to head (71921fd).

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #817   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          383       383           
  Branches         9         9           
=========================================
  Hits           383       383           
Flag Coverage Δ
CI-GHA 100.00% <ø> (ø)
OS-Linux 100.00% <ø> (ø)
OS-Windows 100.00% <ø> (ø)
OS-macOS 100.00% <ø> (ø)
Py-3.10.11 100.00% <ø> (ø)
Py-3.10.20 100.00% <ø> (ø)
Py-3.11.15 100.00% <ø> (ø)
Py-3.11.9 100.00% <ø> (ø)
Py-3.12.10 100.00% <ø> (ø)
Py-3.12.13 100.00% <ø> (ø)
Py-3.13.14 100.00% <ø> (ø)
Py-3.14.6 100.00% <ø> (ø)
Py-3.14.6t 100.00% <ø> (ø)
Py-pypy3.10.16-7.3.19 99.73% <ø> (ø)
VM-macos-latest 100.00% <ø> (ø)
VM-ubuntu-latest 100.00% <ø> (ø)
VM-windows-11-arm 100.00% <ø> (ø)
VM-windows-latest 100.00% <ø> (ø)
pytest 100.00% <ø> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant