Skip to content

⚡️ Speed up function remove_unused_definitions_by_function_names by 15% in PR #935 (fix/ctx-global-definitions-deps) #936

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

⚡️ Speed up function remove_unused_definitions_by_function_names by 15% in PR #935 (fix/ctx-global-definitions-deps) #936

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions codeflash/context/unused_definition_remover.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,10 +447,13 @@ def remove_unused_definitions_recursively( # noqa: PLR0911
new_children = []
section_found_used = False

append_child = new_children.append # Local for speed
# Minimize attribute lookup in loop
rr = remove_unused_definitions_recursively
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad optimization

Copy link
Contributor

@misrasaurabh1 misrasaurabh1 Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not even an attribute

for child in original_content:
filtered, used = remove_unused_definitions_recursively(child, definitions)
filtered, used = rr(child, definitions)
if filtered:
new_children.append(filtered)
append_child(filtered)
section_found_used |= used

if new_children or section_found_used:
Expand Down Expand Up @@ -478,6 +481,12 @@ def collect_top_level_defs_with_usages(
definitions = collect_top_level_definitions(module)

# Collect dependencies between definitions using the visitor pattern

# DependencyCollector uses CST visitor, very expensive! Use only if qualified_function_names is not empty
if not qualified_function_names:
return definitions

# Only instantiate and visit if needed
wrapper = cst.MetadataWrapper(module)
dependency_collector = DependencyCollector(definitions)
wrapper.visit(dependency_collector)
Expand Down Expand Up @@ -510,7 +519,8 @@ def remove_unused_definitions_by_function_names(code: str, qualified_function_na
defs_with_usages = collect_top_level_defs_with_usages(module, qualified_function_names)

# Apply the recursive removal transformation
modified_module, _ = remove_unused_definitions_recursively(module, defs_with_usages)
result = remove_unused_definitions_recursively(module, defs_with_usages)
modified_module = result[0]
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary


return modified_module.code if modified_module else "" # noqa: TRY300
except Exception as e:
Expand Down
Loading