Skip to content

fix: don't discard optimized code when post-split perf estimate fails#451

Merged
dop-amin merged 1 commit into
slothy-optimizer:mainfrom
thisisjube:fix-split-perf-estimate-nonfatal
Jun 17, 2026
Merged

fix: don't discard optimized code when post-split perf estimate fails#451
dop-amin merged 1 commit into
slothy-optimizer:mainfrom
thisisjube:fix-split-perf-estimate-nonfatal

Conversation

@thisisjube

Copy link
Copy Markdown
Contributor

The performance estimate after the split heuristic runs another binary
search (optimize_binsearch), which raises SlothyException on failure
rather than returning success=False. That exception propagated up and
discarded the already-optimized code. It also inherited the optimization
timeout, so a too-short timeout would abort it.

Catch the exception (warn and keep the optimized code without a stall
estimate) and disable the timeout for the estimate, since it only inserts
stalls into the already-fixed ordering.

Closes #96 @dop-amin

@thisisjube thisisjube marked this pull request as draft June 10, 2026 15:54
@thisisjube thisisjube marked this pull request as ready for review June 10, 2026 15:58
@mkannwischer mkannwischer requested a review from dop-amin June 11, 2026 01:18

@dop-amin dop-amin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR!
Functionally, the secondary objective timeout should also be un-set.
From a style point of view, I think getting rid of the duplicate warning and simplifying the code path can be worth it.

Comment thread slothy/core/heuristics.py
Comment thread slothy/core/heuristics.py
@thisisjube thisisjube force-pushed the fix-split-perf-estimate-nonfatal branch from 933f314 to b27c0a5 Compare June 17, 2026 09:30
@thisisjube thisisjube requested a review from dop-amin June 17, 2026 09:31
      The performance estimate after the split heuristic runs another binary
      search (`optimize_binsearch`), which raises `SlothyException` on failure
      rather than returning `success=False`. That exception propagated up and
      discarded the already-optimized code. It also inherited the optimization
      timeout, so a too-short timeout would abort it.

      Catch the exception (warn and keep the optimized code without a stall
      estimate) and disable the timeout for the estimate, since it only inserts
      stalls into the already-fixed ordering.
@thisisjube thisisjube force-pushed the fix-split-perf-estimate-nonfatal branch from b27c0a5 to 1c3ee0a Compare June 17, 2026 10:52

@dop-amin dop-amin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, thanks!

@dop-amin dop-amin merged commit bb7ec95 into slothy-optimizer:main Jun 17, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't abort if split_estimtate_perf fails

2 participants