-
-
Notifications
You must be signed in to change notification settings - Fork 603
⚡️ Speed up function _import_plugin by 39% in PR #4019 (fix-ruff-py310)
#4022
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
⚡️ Speed up function _import_plugin by 39% in PR #4019 (fix-ruff-py310)
#4022
Conversation
The optimized version achieves a **39% speedup** by eliminating unnecessary intermediate dictionary creations and improving iteration patterns. **Key optimizations:** 1. **Eliminated dict comprehensions**: The original code created two intermediate dictionaries (`symbols` dict and filtered `symbols` dict when `__all__` exists). The optimized version iterates directly over module attributes, avoiding these memory allocations. 2. **Cached `module.__dict__` lookup**: Instead of repeatedly accessing `module.__dict__`, it's stored once in `module_dict` and reused. 3. **Optimized `__all__` handling**: When `__all__` exists, the code now iterates through the `__all__` list and does direct dictionary lookups rather than creating a filtered dictionary comprehension. 4. **Early return pattern**: Both branches (with and without `__all__`) now return immediately upon finding the first valid plugin, avoiding unnecessary iterations. **Performance impact by test scenario:** - **Modules with `__all__`**: Dramatic improvements (65-3863% faster) because it avoids creating filtered dictionaries and iterates only over exported names - **Large modules**: Consistent 20-25% speedups due to elimination of dict comprehensions and direct iteration - **Small modules**: 17-22% improvements from reduced overhead - **Edge cases**: Minimal impact (0-3% variation) as expected for simple scenarios The optimization is most effective for modules with many symbols or when `__all__` is present, making it particularly valuable for plugin discovery in large codebases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Summary
Optimizes the _import_plugin function by eliminating unnecessary intermediate dictionary creations, achieving a 39% performance improvement (1.61ms → 1.15ms).
Key changes:
- Caches
module.__dict__lookup to avoid repeated access - When
__all__exists: directly iterates the list and does dictionary lookups instead of creating filtered dict comprehensions - When
__all__doesn't exist: iterates__dict__.items()directly with inline filtering instead of creating intermediatesymbolsdict - Returns immediately upon finding first valid plugin
Performance gains:
- Modules with
__all__: 65-3863% faster (avoids filtered dict creation) - Large modules: 20-25% faster (no dict comprehensions)
- Small modules: 17-22% faster (reduced overhead)
Minor behavioral change:
- Original code filtered out dunder names (
__*) before checking__all__ - New code checks all names in
__all__directly, including dunder names if present - Impact is negligible since dunder attributes aren't plugin classes and won't pass
_is_codegen_pluginchecks
Confidence Score: 4/5
- This PR is safe to merge with low risk - the optimization is sound and well-tested
- The optimization eliminates unnecessary dict comprehensions and achieves significant performance gains. The code is functionally correct, backed by 31 generated regression tests with 90% coverage. Minor behavioral change (checking dunder names in
__all__) has negligible practical impact since dunder attributes won't pass plugin validation anyway - No files require special attention - single file change with straightforward optimization
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| strawberry/cli/commands/codegen.py | 4/5 | Performance optimization of _import_plugin function with a minor behavioral change when __all__ contains dunder names |
Sequence Diagram
sequenceDiagram
participant Caller
participant _import_plugin
participant importlib
participant module
participant _is_codegen_plugin
Caller->>_import_plugin: plugin string
alt plugin contains ":"
_import_plugin->>_import_plugin: split into module_name:symbol_name
end
_import_plugin->>importlib: import_module(module_name)
alt ModuleNotFoundError
importlib-->>_import_plugin: None
_import_plugin-->>Caller: None
else Module found
importlib-->>_import_plugin: module
alt symbol_name provided
_import_plugin->>module: getattr(symbol_name)
module-->>_import_plugin: obj
_import_plugin->>_is_codegen_plugin: check obj
_is_codegen_plugin-->>_import_plugin: True/False (assert)
_import_plugin-->>Caller: obj
else No symbol_name (search module)
_import_plugin->>module: get __dict__ and __all__
alt __all__ exists (NEW: direct iteration)
loop for name in __all__
_import_plugin->>module: get(name)
module-->>_import_plugin: obj or None
alt obj exists
_import_plugin->>_is_codegen_plugin: check obj
alt is valid plugin
_import_plugin-->>Caller: obj
end
end
end
else No __all__ (NEW: iterate __dict__)
loop for key, value in __dict__
alt not key.startswith("__")
_import_plugin->>_is_codegen_plugin: check value
alt is valid plugin
_import_plugin-->>Caller: value
end
end
end
end
_import_plugin-->>Caller: None (no plugin found)
end
end
1 file reviewed, 1 comment
| if all_names is not None: | ||
| for name in all_names: | ||
| obj = module_dict.get(name, None) | ||
| if obj is not None and _is_codegen_plugin(obj): | ||
| return obj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: behavioral change: now checks dunder names if present in __all__
Original filtered out __* names before checking __all__, new code checks all names in __all__ directly. If __all__ contains dunder names (unlikely but possible), they'll now be checked. Won't cause issues since dunder attributes aren't plugin classes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: strawberry/cli/commands/codegen.py
Line: 45:49
Comment:
**style:** behavioral change: now checks dunder names if present in `__all__`
Original filtered out `__*` names before checking `__all__`, new code checks all names in `__all__` directly. If `__all__` contains dunder names (unlikely but possible), they'll now be checked. Won't cause issues since dunder attributes aren't plugin classes.
How can I resolve this? If you propose a fix, please make it concise.
⚡️ This pull request contains optimizations for PR #4019
If you approve this dependent PR, these changes will be merged into the original PR branch
fix-ruff-py310.📄 39% (0.39x) speedup for
_import_plugininstrawberry/cli/commands/codegen.py⏱️ Runtime :
1.61 milliseconds→1.15 milliseconds(best of88runs)📝 Explanation and details
The optimized version achieves a 39% speedup by eliminating unnecessary intermediate dictionary creations and improving iteration patterns.
Key optimizations:
Eliminated dict comprehensions: The original code created two intermediate dictionaries (
symbolsdict and filteredsymbolsdict when__all__exists). The optimized version iterates directly over module attributes, avoiding these memory allocations.Cached
module.__dict__lookup: Instead of repeatedly accessingmodule.__dict__, it's stored once inmodule_dictand reused.Optimized
__all__handling: When__all__exists, the code now iterates through the__all__list and does direct dictionary lookups rather than creating a filtered dictionary comprehension.Early return pattern: Both branches (with and without
__all__) now return immediately upon finding the first valid plugin, avoiding unnecessary iterations.Performance impact by test scenario:
__all__: Dramatic improvements (65-3863% faster) because it avoids creating filtered dictionaries and iterates only over exported namesThe optimization is most effective for modules with many symbols or when
__all__is present, making it particularly valuable for plugin discovery in large codebases.✅ Correctness verification report:
🌀 Generated Regression Tests and Runtime
To edit these changes
git checkout codeflash/optimize-pr4019-2025-10-12T12.27.08and push.