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

isolate not killing immediately after time limit #202

Closed
shaunren opened this issue Nov 5, 2013 · 4 comments
Closed

isolate not killing immediately after time limit #202

shaunren opened this issue Nov 5, 2013 · 4 comments

Comments

@shaunren
Copy link

shaunren commented Nov 5, 2013

Is it possible to make isolate kill the process immediately after timeout? Currently it waits up to one second before killing it, due to alarm(1).

@stefano-maggiolo
Copy link
Member

I admit I'm not very familiar with that code, so I may be mistaken. It seems that alarm(1) means that the time limit check is done every second; isolate kills the subprocess right after having made sure that it passed the timeout.

You could reduce that 1 to something smaller to avoid that potential latency, but I think that the authors of isolate used that relatively long interval because they didn't want to influence too much the timing of the subprocess. Of course wall clock time is influenced, but also the real time could be due to additional context switches. The check for the real time in the worst case [1] seems relatively heavy, so that could matter at least for wall clock time.

[1] https://github.com/cms-dev/cms/blob/master/isolate/isolate.c#L1026

Just to avoid misunderstandings, are you aware that the hard limit after which we definitely kill the subprocess is 2 * timeout + 1 seconds, as stated in [2]? So in the worst case, you are going to wait 2 * timeout + 2 seconds, including the latency in the timeout check.

[2] https://github.com/cms-dev/cms/blob/master/cms/grading/__init__.py#L306

A potentially better idea could be to increase the 1 to 1.1 - usually timeouts are at low integer numbers. Changing from 1 to 1.1 would reduce the average latency for timing out programs from 0.5 to 0.1.

@bblackham what do you think?

@shaunren
Copy link
Author

shaunren commented Nov 8, 2013

man alarm says that it only take seconds as input, and it is unsigned int; therefore, the minimum interval is 1s.

I am aware that cms kills after 2*timeout + 1 seconds. However, if there are a lot of test cases (say 20), each with 1s timeout, up to 20 seconds of the worker's time will be wasted, which is enough to mark another solution.

@stefano-maggiolo
Copy link
Member

It seems you can use also setitimer to fire sigalarm, see e.g. http://www.gnu.org/software/libc/manual/html_node/Setting-an-Alarm.html

@lw
Copy link
Member

lw commented Aug 8, 2014

Moved to cms-dev/isolate#8.

@lw lw closed this as completed Aug 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants