Repository navigation
Avoid allocating a result vector for a single ctypes output - #9749
Merged
youknowone merged 1 commit intoNov 11, 2026
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. Pre-merge checks:
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check |  |
This assessment has not completed. The check will be retried on the next pre-merge check run. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Keep OutValue and all owners alive across output conversion. Exercise the real VM/libffi return path with a native test fixture, without enabling the separately incomplete DLL paramflags constructor. Assisted-by: OpenAI Codex:model version not exposed
youknowdot
force-pushed
the
perf/ctypes-single-output-20261108
branch
from
November 11, 2026 03:16
bf7162b to
5aaaf67
Compare
youknowone
approved these changes
Nov 11, 2026
youknowone
marked this pull request as ready for review
November 11, 2026 14:07
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
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.
Summary
Follow up on #9618 without changing its
OutValuerepresentation or object ownership.For one OUT/INOUT result, convert/clone the borrowed value directly instead of collecting it into a temporary Vec and immediately extracting it. Multiple outputs still collect into a tuple. Raw-result disposal, errcheck short-circuiting, callback order, exception propagation, and the lifetime of all output owners are unchanged.
This follows the singleton-result shape of CPython 3.15
_build_result. It is not a bitmask redesign or a claim of complete ctypes compatibility.Real native-call regression test
The existing ordinary DLL constructor discards
paramflags, so using that constructor would not exercise this path. The test fixture sets only the internal parameter metadata (as the COM constructor does), then executes Python calls through the actual VM and libffi to small native C-ABI functions.It checks single custom OUT conversion, original INOUT identity, multiple outputs, raw-result disposal order, errcheck replacement/identity, and conversion exceptions. It does not assert source tokens or macro expansion.
The separate DLL-constructor/resolved-callargs/errcheck-argument behavior is deliberately not enabled or changed here.
Original validation (before the November 11 rebase)
Passed on Linux:
cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capicargo test--all-targets -- -D warnings-m test test_ctypes: SUCCESS (run=323,skipped=56)extra_tests/snippets/stdlib_ctypes.pyin debug and releaseNo throughput speedup percentage is claimed; this removes the singleton result-vector construction, not all allocations in a ctypes call. Windows/macOS CI remains to be checked.
November 11 rebase
Rebased independently onto
dd8c34399621e60e703caeff89b2d5ae5c2f64f6without changing the patch (git range-diffreports equality). Changed-file pre-commit hooks passed. Fresh CI for this head and local full validation are pending.The previous failed jobs stopped during external tool setup: HTTP 504 downloading Python, prek/reviewdog, and CodSpeed, and a missing Wasmer executable. None reported a failure in the changed ctypes code.
AI assistance: OpenAI Codex assisted with implementation, testing, review, and the rebase.