ichthyo/LUMIERA.clone
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

DispatcherLoop: no timeout turnaround necessary in idle state

Browse source 
...since the session loop will be notified on any change via the
interface, adding a command will activate the loop, and the builder
timeout is handled separately via the dirty state. So there is no
need to spin around the loop in idle state.

As a aside, timeout waiting on a condition variable can be intentional
and should thus not be logged as an error automatically. It is up to the
calling context to decide if a timeout constitutes an exceptional situation.

It is always a trade-off performance vs. readability.
Sometimes a single-threaded implementation of self-contained logic
is preferable to a slightly more performant yet obscure implementation
based on our threadpool and scheduler.
This commit is contained in:
Fischlurch 2017-01-07 02:46:34 +01:00
parent dd041ff80c
commit 2535e1b554
4 changed files with 17 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/lib/error.h
View file

@ -128,7 +128,7 @@ lumiera_err LUMIERA_ERROR_##err = "LUMIERA_ERROR_" #err ":" msg
/**
* Helper macro to raise an error for the current thread.
* Same as LUMIERA_ERROR_SET(), but logs at 'LOG_WARNING' level.
* Use this when a not unexected error happens which can be handled.
* Use this when a not unexpected error happens which can be handled.
* @param flag NoBug flag describing the subsystem where the error was raised
* @param err name of the error without the 'LUMIERA_ERROR_' prefix (example: NO_MEMORY)
* @param extra optional string (or NULL) which adds some more context to the error, can be a temporary

7
src/lib/lockerror.c
View file

@ -47,6 +47,10 @@ lumiera_lockerror_set (int err, struct nobug_flag* flag, const struct nobug_cont
{
case 0:
break;
case ETIMEDOUT:
lumiera_error_set(LUMIERA_ERROR_LOCK_TIMEOUT, ctx.func);
// no implicit logging, since timeout can be intentional
break;
case EINVAL:
LUMIERA_ERROR_SET_ALERT(NOBUG_FLAG_RAW(flag), LOCK_INVAL, ctx.func);
break;
@ -59,9 +63,6 @@ lumiera_lockerror_set (int err, struct nobug_flag* flag, const struct nobug_cont
case EPERM:
LUMIERA_ERROR_SET_ALERT(NOBUG_FLAG_RAW(flag), LOCK_PERM, ctx.func);
break;
case ETIMEDOUT:
LUMIERA_ERROR_SET(NOBUG_FLAG_RAW(flag), LOCK_TIMEOUT, ctx.func);
break;
case EAGAIN:
LUMIERA_ERROR_SET_WARNING(NOBUG_FLAG_RAW(flag), LOCK_AGAIN, ctx.func);
break;

2
src/proc/control/looper.hpp
View file

@ -207,7 +207,7 @@ namespace control {
ulong /////////////////////////////////////////////TICKET #1056 : better return a std::chrono value here
getTimeout() const
{
if (isDisabled())
if (isDisabled() or not dirty_)
return 0;
else
return wakeTimeout_ms()

18
tests/core/proc/control/dispatcher-looper-test.cpp
View file

@ -124,6 +124,10 @@ namespace test {
CHECK (not looper.isDying());
CHECK (looper.shallLoop());
CHECK (not looper.runBuild());
CHECK (isDisabled (looper.getTimeout()));
setup.has_commands_in_queue = true;
CHECK (looper.requireAction());
uint timeout = looper.getTimeout();
CHECK (10 < timeout, "configured idle timeout %2u to short", timeout);
@ -397,7 +401,7 @@ namespace test {
CHECK (not looper.runBuild()); // ...note: now done with building
CHECK ( looper.isIdle());
CHECK (isSlow (looper.getTimeout())); // ...now Dispatcher is idle and goes to sleep
CHECK (isDisabled(looper.getTimeout())); // ...now Dispatcher is idle and goes to sleep
setup.has_commands_in_queue = true; // next command pending
@ -425,7 +429,7 @@ namespace test {
CHECK (not looper.requireAction());
CHECK ( looper.isDisabled());
CHECK (not looper.isWorking());
CHECK (not looper.runBuild()); // ...note: dirty state hidden by disabled state
CHECK (not looper.runBuild()); // ...note: dirty state hidden by disabled state
CHECK (not looper.isIdle());
CHECK (isDisabled (looper.getTimeout()));
@ -436,7 +440,7 @@ namespace test {
CHECK (not looper.requireAction());
CHECK (not looper.isDisabled());
CHECK (not looper.isWorking());
CHECK ( looper.runBuild()); // ...note: dirty state revealed again
CHECK ( looper.runBuild()); // ...note: dirty state revealed again
CHECK (not looper.isIdle());
CHECK (isFast (looper.getTimeout()));
@ -447,7 +451,7 @@ namespace test {
CHECK (not looper.requireAction());
CHECK ( looper.isDisabled());
CHECK (not looper.isWorking());
CHECK (not looper.runBuild()); // ...note: dirty state not obvious
CHECK (not looper.runBuild()); // ...note: dirty state not obvious
CHECK (not looper.isIdle());
CHECK (isDisabled (looper.getTimeout()));
@ -458,10 +462,10 @@ namespace test {
CHECK (not looper.requireAction());
CHECK (not looper.isDisabled());
CHECK (not looper.isWorking());
CHECK (not looper.runBuild()); // ...note: but now it becomes clear builder is not dirty
CHECK (not looper.runBuild()); // ...note: but now it becomes clear builder is not dirty
CHECK ( looper.isIdle());
CHECK (isSlow (looper.getTimeout()));
CHECK (isDisabled (looper.getTimeout()));
setup.has_commands_in_queue = true; // more commands again
@ -490,7 +494,7 @@ namespace test {
CHECK ( looper.requireAction());
CHECK ( looper.isDisabled());
CHECK (not looper.isWorking());
CHECK (not looper.runBuild()); // ...note: still no need for builder run, since in shutdown
CHECK (not looper.runBuild()); // ...note: still no need for builder run, since in shutdown
CHECK (not looper.isIdle());
CHECK (isDisabled (looper.getTimeout()));