[codex] optimize hot AST passes - #12
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b015bb91ba
ℹ️ 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".
| self.callable_argument_mappings[old_name] = copied | ||
| self.callable_argument_mappings[new_name] = copied |
There was a problem hiding this comment.
Track argument mappings for callable aliases
When rename_arguments=True, argument rewrites are only registered for a function's original and renamed identifiers, so calls through aliases are missed. For example, alias = square; alias(long_value=3) is minified to a call that still uses long_value even though square's parameter was renamed, which raises TypeError at runtime. Alias assignments need to propagate the same callable-argument mapping to preserve keyword-call semantics.
Useful? React with 👍 / 👎.
| if base_name in self.class_method_argument_mappings: | ||
| return self.class_method_argument_mappings[base_name].get(func.attr) |
There was a problem hiding this comment.
Resolve method keyword mappings on typed instances
The method keyword rewrite path only checks class contexts and direct class-name receivers, but it does not use instance bindings for obj.method(...) calls. With rename_arguments=True, code like m = Math(); m.square(long_value=3) keeps the old keyword after square's parameter is renamed, causing an unexpected-keyword TypeError in the minified output. The argument mapping lookup should also consult _lookup_instance_type(base_name) before giving up.
Useful? React with 👍 / 👎.
Summary
pymini's hottest AST passes without changing output size or semanticsmapping.values()scans and unnecessarygeneric_visit()calls onast.NameWhy
The previous speed pass fixed the largest structural regressions, but package-mode minification still spent a lot of time rewalking ASTs to recompute reserved names, scope bindings, and public-member profitability. This follow-up cuts those repeated walks while keeping compression rate and correctness intact.
Impact
Current benchmark results on this branch:
pyminifier.py:1.5 mspyminify.py:4.1 msTexSoup/*.py:103.9 msTexSoupCLI package mode:105.1 msCompared to the start of this branch:
pyminifier.py:1.7 ms -> 1.5 mspyminify.py:5.0 ms -> 4.1 msTexSoup/*.py:162.1 ms -> 103.9 msValidation
PYTHONPATH=. .venv/bin/python -m pytestPYTHONPATH=. .venv/bin/python scripts/regenerate_examples.py --checkPYTHONPATH=. .venv/bin/python -m pymini package /private/tmp/pymini-texsoup-repo/TexSoup -o /private/tmp/pymini-texsoup-testtargetPYTHONPATH=/private/tmp/pymini-texsoup-testtarget:/private/tmp/pymini-texsoup-repo/tests .venv/bin/python -m pytest /private/tmp/pymini-texsoup-repo/tests -o addopts=''PYTHONPATH=. .venv/bin/python benchmarks/benchmark_speed.py --pyminifier-root /private/tmp/pymini-pyminifier-src/pyminifier-2.1