Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Dec 14, 2025

PR Type

Enhancement, Tests


Description

  • Merge rank/filter into rank_functions

  • File-relative importance filtering added

  • Simplify replay test generation API

  • Minor compatibility and safety tweaks


Diagram Walkthrough

flowchart LR
  ranker["FunctionRanker: consolidated ranking/filtering"] -- "uses" --> fileRel["File-relative importance calc"]
  ranker -- "sort by" --> ttx["ttX score accessor"]
  tracingReplay["Tracing replay test generator"] -- "drop unittest support" --> pytestOnly["Pytest-only tests"]
  tracingProcess["Tracer new process"] -- "derive output path" --> testsDir["Tests directory (replay)"]
  tracingProcess -- "generate tests" --> tracingReplay
  testsUpdated["Tests updated"] -- "assert filtered+ranked" --> ranker
Loading

File Walkthrough

Relevant files
Enhancement
function_ranker.py
Consolidate ranking and add file-relative importance         

codeflash/benchmarking/function_ranker.py

  • Rename _get_function_stats to get_function_stats_summary.
  • Consolidate filter+rank into rank_functions.
  • Compute importance relative to target files.
  • Improve logging and keep ttX-based sorting.
+59/-48 
replay_test.py
Simplify replay test generation to pytest-only                     

codeflash/benchmarking/replay_test.py

  • Remove unittest/pytest switch; default to pytest style.
  • Simplify imports and test template generation.
  • Adjust function signatures and return docs.
  • Uniform 4-space indentation for tests.
+7/-21   
replay_test.py
Streamline tracing replay to pytest templates                       

codeflash/tracing/replay_test.py

  • Remove unittest option and assertions.
  • Simplify imports and test scaffolding.
  • Standardize function signature and indentation.
  • Generate standalone pytest tests.
+5/-15   
tracing_new_process.py
Align tracer outputs with replay tests and safety tweaks 

codeflash/tracing/tracing_new_process.py

  • Derive trace output path beside replay tests.
  • Remove reliance on config test_framework.
  • Pass simplified params to replay generator.
  • Safeguard dict iteration in pstats conversion.
+10/-5   
Tests
test_function_ranker.py
Update tests for consolidated ranking behavior                     

tests/test_function_ranker.py

  • Update tests to new rank_functions semantics.
  • Remove rerank_and_filter_functions test.
  • Add assertions for filtering and ordering.
+7/-12   

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

In file-relative total time calculation, membership check uses substring matching against s["filename"]; this can misattribute functions when filenames overlap (e.g., foo.py vs myfoo.py). Consider stricter path matching to avoid incorrect importance filtering.

target_files = {func.file_path.name for func in functions_to_optimize}
# Calculate total time only from functions in these files
total_program_time = sum(
    s["own_time_ns"]
    for s in self._function_stats.values()
    if s.get("own_time_ns", 0) > 0 and any(target_file in s["filename"] for target_file in target_files)
)
logger.debug(
Output Path Logic

Tracer now derives output_file based on tests_root via get_test_file_path; if config["tests_root"] is missing/invalid or functions is long, path resolution could fail or create overly long filenames. Validate existence, handle exceptions, and possibly truncate/sanitize function_path.

# Place trace file next to replay tests in the tests directory
from codeflash.verification.verification_utils import get_test_file_path
function_path = "_".join(functions) if functions else self.sanitized_filename
test_file_path = get_test_file_path(
    test_dir=Path(config["tests_root"]), function_name=function_path, test_type="replay"
)
trace_filename = test_file_path.stem + ".trace"
self.output_file = test_file_path.parent / trace_filename
Iteration Safety

make_pstats_compatible now wraps dict iteration with list(...), which is good; ensure large dicts don’t cause memory spikes and consider using tuple(...) or copying keys only if size is significant.

def make_pstats_compatible(self) -> None:
    # delete the extra class_name item from the function tuple
    self.files = []
    self.top_level = []
    new_stats = {}
    for func, (cc, ns, tt, ct, callers) in list(self.stats.items()):
        new_callers = {(k[0], k[1], k[2]): v for k, v in callers.items()}
        new_stats[(func[0], func[1], func[2])] = (cc, ns, tt, ct, new_callers)
    new_timings = {}
    for func, (cc, ns, tt, ct, callers) in list(self.timings.items()):
        new_callers = {(k[0], k[1], k[2]): v for k, v in callers.items()}
        new_timings[(func[0], func[1], func[2])] = (cc, ns, tt, ct, new_callers)
    self.stats = new_stats
    self.timings = new_timings

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Create output directory proactively

Ensure the output directory exists before assigning self.output_file. Without
creating the parent directory, subsequent writes can fail with FileNotFoundError
when the tests directory tree is missing.

codeflash/tracing/tracing_new_process.py [131-135]

 self.sanitized_filename = self.sanitize_to_filename(command)
 # Place trace file next to replay tests in the tests directory
 from codeflash.verification.verification_utils import get_test_file_path
 function_path = "_".join(functions) if functions else self.sanitized_filename
 test_file_path = get_test_file_path(
     test_dir=Path(config["tests_root"]), function_name=function_path, test_type="replay"
 )
+test_file_path.parent.mkdir(parents=True, exist_ok=True)
 trace_filename = test_file_path.stem + ".trace"
 self.output_file = test_file_path.parent / trace_filename
Suggestion importance[1-10]: 8

__

Why: Ensuring the parent directory exists before writing avoids a likely FileNotFoundError in new setups; this is a practical reliability fix with high impact and aligns with the new path logic.

Medium
Safely handle missing filenames

Guard against missing or None filename in stats to avoid TypeError when using in.
Use str(...) or an .get() default to ensure safe substring checks. This prevents
crashes when profile entries lack a filename.

codeflash/benchmarking/function_ranker.py [128-132]

 total_program_time = sum(
     s["own_time_ns"]
     for s in self._function_stats.values()
-    if s.get("own_time_ns", 0) > 0 and any(target_file in s["filename"] for target_file in target_files)
+    if s.get("own_time_ns", 0) > 0
+    and any(target_file in str(s.get("filename", "")) for target_file in target_files)
 )
Suggestion importance[1-10]: 7

__

Why: Correctly guards against missing filename by coercing to string, preventing a potential TypeError during substring checks; impact is moderate as it improves robustness without changing behavior otherwise.

Medium
General
Remove unintended console output

Avoid using console.rule() unconditionally, as it performs I/O during ranking and
can break in non-console environments. Remove it or guard behind a verbosity flag to
prevent unintended side effects in library code.

codeflash/benchmarking/function_ranker.py [163-164]

 if total_program_time == 0:
     logger.warning("Total program time is zero, cannot determine function importance.")
     functions_to_rank = functions_to_optimize
 else:
     functions_to_rank = []
     for func in functions_to_optimize:
         func_stats = self.get_function_stats_summary(func)
         if func_stats and func_stats.get("own_time_ns", 0) > 0:
             importance = func_stats["own_time_ns"] / total_program_time
             if importance >= DEFAULT_IMPORTANCE_THRESHOLD:
                 functions_to_rank.append(func)
             else:
                 logger.debug(
                     f"Filtering out function {func.qualified_name} with importance "
                     f"{importance:.2%} (below threshold {DEFAULT_IMPORTANCE_THRESHOLD:.2%})"
                 )
 
     logger.info(
         f"Filtered down to {len(functions_to_rank)} important functions "
         f"from {len(functions_to_optimize)} total functions"
     )
-    console.rule()
Suggestion importance[1-10]: 5

__

Why: Removing console.rule() reduces unnecessary I/O in library code; it's a minor quality improvement and context-dependent, so moderate impact.

Low

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 14, 2025

⚡️ Codeflash found optimizations for this PR

📄 2,062% (20.62x) speedup for FunctionRanker.get_function_stats_summary in codeflash/benchmarking/function_ranker.py

⏱️ Runtime : 1.48 milliseconds 68.6 microseconds (best of 115 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch ranking-changes).

Static Badge

The optimization replaces an O(N) linear search through all functions with an O(1) hash table lookup followed by iteration over only matching function names.

**Key Changes:**
- Added `_function_stats_by_name` index in `__init__` that maps function names to lists of (key, stats) tuples
- Modified `get_function_stats_summary` to first lookup candidates by function name, then iterate only over those candidates

**Why This is Faster:**
The original code iterates through ALL function stats (22,603 iterations in the profiler results) for every lookup. The optimized version uses a hash table to instantly find only the functions with matching names, then iterates through just those candidates (typically 1-2 functions).

**Performance Impact:**
- **Small datasets**: 15-30% speedup as shown in basic test cases
- **Large datasets**: Dramatic improvement - the `test_large_scale_performance` case with 900 functions shows **3085% speedup** (66.7μs → 2.09μs)
- **Overall benchmark**: 2061% speedup demonstrates the optimization scales excellently with dataset size

**When This Optimization Shines:**
- Large codebases with many profiled functions (where the linear search becomes expensive)
- Repeated function lookups (if this method is called frequently)
- Cases with many unique function names but few duplicates per name

The optimization maintains identical behavior while transforming the algorithm from O(N) per lookup to O(average functions per name) per lookup, which is typically O(1) in practice.

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>
@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 14, 2025

This PR is now faster! 🚀 @KRRT7 accepted my optimizations from:

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 14, 2025

This PR is now faster! 🚀 codeflash-ai[bot] accepted my code suggestion above.

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 14, 2025

⚡️ Codeflash found optimizations for this PR

📄 26% (0.26x) speedup for FunctionRanker.rank_functions in codeflash/benchmarking/function_ranker.py

⏱️ Runtime : 42.5 milliseconds 33.8 milliseconds (best of 57 runs)

A new Optimization Review has been created.

🔗 Review here

Static Badge

KRRT7 and others added 10 commits December 16, 2025 14:41
The optimization applies **local variable caching** to eliminate repeated attribute lookups on `self.test_result_idx` and `self.test_results`. 

**Key Changes:**
- Added `test_result_idx = self.test_result_idx` and `test_results = self.test_results` to cache references locally
- Used these local variables instead of accessing `self.*` attributes multiple times

**Why This Works:**
In Python, attribute access (e.g., `self.test_result_idx`) involves dictionary lookups in the object's `__dict__`, which is slower than accessing local variables. By caching these references, we eliminate redundant attribute resolution overhead on each access.

**Performance Impact:**
The line profiler shows the optimization reduces total execution time from 12.771ms to 19.482ms in the profiler run, but the actual runtime improved from 2.13ms to 1.89ms (12% speedup). The test results consistently show 10-20% improvements across various scenarios, particularly benefiting:
- Large-scale operations (500+ items): 14-16% faster
- Multiple unique additions: 15-20% faster  
- Mixed workloads with duplicates: 7-15% faster

**Real-World Benefits:**
This optimization is especially valuable for high-frequency test result collection scenarios where the `add` method is called repeatedly in tight loops, as the cumulative effect of eliminating attribute lookups becomes significant at scale.
…lts.add-mj98n62n

⚡️ Speed up method `TestResults.add` by 12%
@KRRT7 KRRT7 requested a review from misrasaurabh1 December 17, 2025 19:27
* Optimize get_cached_gh_event_data


The optimization replaces `Path(event_path).open(encoding="utf-8")` with the built-in `open(event_path, encoding="utf-8")`, achieving a **12% speedup** by eliminating unnecessary object allocation overhead.

**Key optimization:**
- **Removed Path object creation**: The original code creates a `pathlib.Path` object just to call `.open()` on it, when the built-in `open()` function can directly accept the string path from `event_path`.
- **Reduced memory allocation**: Avoiding the intermediate `Path` object saves both allocation time and memory overhead.

**Why this works:**
In Python, `pathlib.Path().open()` internally calls the same file opening mechanism as the built-in `open()`, but with additional overhead from object instantiation and method dispatch. Since `event_path` is already a string from `os.getenv()`, passing it directly to `open()` is more efficient.

**Performance impact:**
The test results show consistent improvements across all file-reading scenarios:
- Simple JSON files: 12-20% faster
- Large files (1000+ elements): 3-27% faster  
- Error cases (missing files): Up to 71% faster
- The cached calls remain unaffected (0% change as expected)

**Workload benefits:**
Based on the function references, `get_cached_gh_event_data()` is called by multiple GitHub-related utility functions (`get_pr_number()`, `is_repo_a_fork()`, `is_pr_draft()`). While the `@lru_cache(maxsize=1)` means the file is only read once per program execution, this optimization reduces the initial cold-start latency for GitHub Actions workflows or CI/CD pipelines where these functions are commonly used.

The optimization is particularly effective for larger JSON files and error handling scenarios, making it valuable for robust CI/CD environments that may encounter various file conditions.

* ignore

---------

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>
Co-authored-by: Kevin Turcios <turcioskevinr@gmail.com>
@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 17, 2025

This PR is now faster! 🚀 codeflash-ai[bot] accepted my code suggestion above.

* Optimize function_is_a_property


The optimized version achieves a **60% speedup** by replacing Python's `any()` generator expression with a manual loop and making three key micro-optimizations:

**What was optimized:**
1. **Replaced `isinstance()` with `type() is`**: Direct type comparison (`type(node) is ast_Name`) is faster than `isinstance(node, ast.Name)` for AST nodes where subclassing is rare
2. **Eliminated repeated lookups**: Cached `"property"` as `property_id` and `ast.Name` as `ast_Name` in local variables to avoid global/attribute lookups in the loop
3. **Manual loop with early return**: Replaced `any()` generator with explicit `for` loop that returns `True` immediately upon finding a match, avoiding generator overhead

**Why it's faster:**
- The `any()` function creates generator machinery that adds overhead, especially for small decorator lists
- `isinstance()` performs multiple checks while `type() is` does a single identity comparison
- Local variable access is significantly faster than repeated global/attribute lookups in tight loops

**Performance characteristics from tests:**
- **Small decorator lists** (1-3 decorators): 50-80% faster due to reduced per-iteration overhead
- **Large decorator lists** (1000+ decorators): 55-60% consistent speedup, with early termination providing additional benefits when `@property` appears early
- **Empty decorator lists**: 77% faster due to avoiding `any()` generator setup entirely

**Impact on workloads:**
Based on the function references, this function is called during AST traversal in `visit_FunctionDef` and `visit_AsyncFunctionDef` methods - likely part of a code analysis pipeline that processes many functions. The 60% speedup will be particularly beneficial when analyzing codebases with many decorated functions, as this optimization reduces overhead in a hot path that's called once per function definition.

* format

---------

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>
Co-authored-by: Kevin Turcios <turcioskevinr@gmail.com>
@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 17, 2025

This PR is now faster! 🚀 codeflash-ai[bot] accepted my code suggestion above.

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 17, 2025

⚡️ Codeflash found optimizations for this PR

📄 11% (0.11x) speedup for function_is_a_property in codeflash/discovery/functions_to_optimize.py

⏱️ Runtime : 1.13 milliseconds 1.02 milliseconds (best of 90 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch ranking-changes).

Static Badge

The optimization achieves an **11% speedup** through two key changes:

**1. Constant Hoisting:** The original code repeatedly assigns `property_id = "property"` and `ast_name = ast.Name` on every function call. The optimized version moves these to module-level constants `_property_id` and `_ast_name`, eliminating 4,130 redundant assignments per profiling run (saving ~2.12ms total time).

**2. isinstance() vs type() comparison:** Replaced `type(node) is ast_name` with `isinstance(node, _ast_name)`. While both are correct for AST nodes (which use single inheritance), `isinstance()` is slightly more efficient for type checking in Python's implementation.

**Performance Impact:** The function is called in AST traversal loops when discovering functions to optimize (`visit_FunctionDef` and `visit_AsyncFunctionDef`). Since these visitors process entire codebases, the 11% per-call improvement compounds significantly across large projects.

**Test Case Performance:** The optimization shows consistent gains across all test scenarios:
- **Simple cases** (no decorators): 29-42% faster due to eliminated constant assignments
- **Property detection cases**: 11-26% faster from combined optimizations  
- **Large-scale tests** (500-1000 functions): 18.5% faster, demonstrating the cumulative benefit when processing many functions

The optimizations are particularly effective for codebases with many function definitions, where this function gets called repeatedly during AST analysis.

Co-authored-by: codeflash-ai[bot] <148906541+codeflash-ai[bot]@users.noreply.github.com>
@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 17, 2025

This PR is now faster! 🚀 @KRRT7 accepted my optimizations from:

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 17, 2025

This PR is now faster! 🚀 codeflash-ai[bot] accepted my code suggestion above.

@codeflash-ai
Copy link
Contributor

codeflash-ai bot commented Dec 17, 2025

⚡️ Codeflash found optimizations for this PR

📄 19% (0.19x) speedup for get_cached_gh_event_data in codeflash/code_utils/env_utils.py

⏱️ Runtime : 1.44 milliseconds 1.22 milliseconds (best of 250 runs)

A dependent PR with the suggested changes has been created. Please review:

If you approve, it will be merged into this PR (branch ranking-changes).

Static Badge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants