Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented May 20, 2025

User description

Current loop is structured as an infinite loop which terminates when certain conditions are met, it seems at the moment that there aren't any edge cases which could lead to an infinite loop. I am creating this PR just in case we encounter such a case.


PR Type

Bug fix


Description

  • Replace infinite loop with conditional queue

  • Remove IndexError exception for empty deque

  • Wait for pending line profiler futures

  • Update candidate count and logging


Changes walkthrough 📝

Relevant files
Error handling
function_optimizer.py
Refactor candidate processing loop                                             

codeflash/optimization/function_optimizer.py

  • Changed while True to while candidates
  • Removed try/except for IndexError on pop
  • Added wait for pending profiler futures
  • Extended candidates and updated logs
  • +11/-10 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    github-actions bot commented May 20, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 9b6dfc4)

    Here are some key observations to aid the review process:

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

    Undefined Variable

    The variable done may not be initialized if candidates is empty before entering the while loop, leading to a NameError when it's referenced later.

    if (not len(candidates)) and (not done):  # all original candidates processed but lp results haven't been processed
    Blocking Wait

    Using concurrent.futures.wait without a timeout can block indefinitely if the future never completes, consider adding a timeout or handling potential hang scenarios.

    concurrent.futures.wait([future_line_profile_results])

    @github-actions
    Copy link

    github-actions bot commented May 20, 2025

    PR Code Suggestions ✨

    Latest suggestions up to 9b6dfc4
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Include pending profiler futures

    The loop will exit prematurely if candidates is empty but
    future_line_profile_results still has pending data. Extend the condition to continue
    looping until all profiler futures have been processed.

    codeflash/optimization/function_optimizer.py [395]

    -while candidates:
    +while candidates or future_line_profile_results is not None:
    Suggestion importance[1-10]: 4

    __

    Why: The new block at lines 514–522 already handles pending futures after the loop, making this change redundant and of moderate impact.

    Low
    General
    Simplify done flag assignment

    This ternary can be simplified for clarity and readability by using a boolean
    expression directly.

    codeflash/optimization/function_optimizer.py [396]

    -done = True if future_line_profile_results is None else future_line_profile_results.done()
    +done = future_line_profile_results is None or future_line_profile_results.done()
    Suggestion importance[1-10]: 3

    __

    Why: This is a valid readability improvement for the ternary expression but has only minor impact on functionality.

    Low

    Previous suggestions

    Suggestions up to commit 1b13456
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    use explicit loop condition

    Replace the unconditional while True loop with a loop that explicitly checks the
    completion condition, making the exit criterion clear and preventing accidental
    infinite loops. This change increases readability and safety by enforcing a single
    loop condition.

    codeflash/optimization/function_optimizer.py [395-399]

    -# TODO replace while true with something safer, we dont want accidental infinite loops
    -while True:
    -    done = True if future_line_profile_results is None else future_line_profile_results.done()
    -    if done and (future_line_profile_results is not None):
    -        line_profile_results = future_line_profile_results.result()
    +# loop until the future is done or no future provided
    +while future_line_profile_results is not None and not future_line_profile_results.done():
    +    pass
    +if future_line_profile_results is not None:
    +    line_profile_results = future_line_profile_results.result()
    Suggestion importance[1-10]: 8

    __

    Why: The original while True loop can hang indefinitely; using an explicit loop condition tied to future_line_profile_results ensures the loop exits safely and prevents accidental infinite loops.

    Medium

    @aseembits93 aseembits93 marked this pull request as ready for review May 21, 2025 03:31
    @github-actions
    Copy link

    Persistent review updated to latest commit 9b6dfc4

    @aseembits93 aseembits93 requested a review from KRRT7 May 21, 2025 03:52
    @aseembits93
    Copy link
    Contributor Author

    @KRRT7 ready to merge

    @aseembits93 aseembits93 enabled auto-merge May 28, 2025 01:52
    @aseembits93 aseembits93 merged commit 1032ed4 into main May 28, 2025
    16 checks passed
    @aseembits93 aseembits93 deleted the fix-lineprofiler branch May 28, 2025 02:30
    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.

    2 participants