Skip to content
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

Limit wall clock time spent on optimization trials #115

Merged
merged 1 commit into from
Jul 12, 2018

Conversation

kornelski
Copy link
Contributor

For automated optimization tools it's problematic when large images take very long time to optimize, so they have to have some time limit.

Simply killing the process when it takes too long is wasteful. It's better to keep existing work done and exit gracefully.

This PR adds a coarse-grained limit for maximum time spent on trying potential optimizations. For simplicity, it doesn't try to finish exactly within the given timeout, merely stops adding new work after a deadline.

src/lib.rs Outdated
@@ -158,6 +160,10 @@ pub struct Options {
///
/// Default: 1.5x CPU cores, rounded down
pub threads: usize,

/// Maximum amount of time to spend on optimizations.
/// Further potential optimizations are skiped if the timeout is exceeded.
Copy link
Owner

Choose a reason for hiding this comment

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

Typo in "skipped".

struct Deadline {
start: Instant,
timeout: Option<Duration>,
print_message: Mutex<bool>,
Copy link
Owner

Choose a reason for hiding this comment

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

Could this be accomplished with an AtomicBool?

Copy link
Contributor Author

@kornelski kornelski Jul 11, 2018

Choose a reason for hiding this comment

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

Probably, but the implementation would be less obvious. This path is very cold, so it wouldn't make any difference to performance. I'd rather not complicate it.

I'd use std::sync::Once for this if it was usable.

@shssoichiro shssoichiro merged commit 24735b6 into shssoichiro:master Jul 12, 2018
@kornelski kornelski deleted the timeout branch July 12, 2018 13:53
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.

2 participants