perf: avoid sorting normalized ABI inputs - #329
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8c26ed5f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return tuple() | ||
| # Align args according to their position in the ABI. | ||
| args_as_kwargs.update(kwargs) | ||
| return tuple(args_as_kwargs[name] for name in sorted_arg_names) |
There was a problem hiding this comment.
Preserve positional values for duplicate ABI names
When duplicate ABI names occur in the positional prefix—which is an accepted case in ABI_FUNCTION_THREE_NAMED_ARGS_DUPLICATE—a mixed call can silently change argument values. For example, inputs named (b, b, a) with args=(1, 2) and a=3 collapse the positional mapping to b: 2, so this reconstruction returns (2, 2, 3) instead of (1, 2, 3). Construct the result from the untouched positional tuple followed by keyword values for the remaining ABI positions rather than looking every position up by name.
Useful? React with 👍 / 👎.
What I did
Optimized
get_normalized_abi_inputs()to align mixed positional and keyword inputs without sorting intermediate(name, value)pairs.fixes: N/A
How I did it
The function still performs the same count, duplicate, and unknown-keyword validation. After validation, it updates the positional argument mapping with keyword arguments and builds the result tuple directly in ABI input order.
How to verify it
Run
python -m pytest tests/core/abi-utils/test_abi_utils.py,tox -e py310-core, andtox -e py310-lint.Local microbench for mixed positional/keyword input:
1.72 usecper loop on upstream base,1.13 usecper loop on this branch, about 34% faster.Checklist