From 892099412cdf171d9a7f960c4a5e2d2b063bd5fc Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Tue, 7 Nov 2023 18:37:20 +0100 Subject: [PATCH] Scheduler: integrate sanity check on timings ...especially to prevent a deadline way too far into the future, since this would provoke the BlockFlow (epoch based) memory manager to run out of space. Just based on gut feeling, I am now imposing a limit of 20seconds, which, given current parametrisation, with a minimum spacing of 6.6ms and 500 Activities per Block would at maximum require 360 MiB for the Activities, or 3000 Blocks. With *that much* blocks, the linear search would degrade horribly anyway... --- src/vault/gear/scheduler-commutator.hpp | 24 ++++++ .../vault/gear/scheduler-commutator-test.cpp | 17 ++-- tests/vault/gear/scheduler-service-test.cpp | 2 +- wiki/thinkPad.ichthyo.mm | 79 ++++++++++++++++--- 4 files changed, 101 insertions(+), 21 deletions(-) diff --git a/src/vault/gear/scheduler-commutator.hpp b/src/vault/gear/scheduler-commutator.hpp index 9a70cb836..2acd88029 100644 --- a/src/vault/gear/scheduler-commutator.hpp +++ b/src/vault/gear/scheduler-commutator.hpp @@ -76,6 +76,7 @@ #include "vault/gear/scheduler-invocation.hpp" #include "vault/gear/activity-lang.hpp" #include "lib/time/timevalue.hpp" +#include "lib/format-string.hpp" #include "lib/nocopy.hpp" #include @@ -85,12 +86,19 @@ namespace vault{ namespace gear { + using lib::time::Offset; + using lib::time::FSecs; using lib::time::Time; using std::atomic; using std::memory_order::memory_order_relaxed; using std::memory_order::memory_order_acquire; using std::memory_order::memory_order_release; + namespace { // Configuration / Scheduling limit + + Offset FUTURE_PLANNING_LIMIT{FSecs{20}}; ///< limit timespan of deadline into the future (~360 MiB max) + } + /*************************************************************//** @@ -156,6 +164,21 @@ namespace gear { } + void + sanityCheck (ActivationEvent const& event, Time now) + { + if (event.startTime() == Time::ANYTIME) + throw error::Fatal ("Attempt to schedule an Activity without valid start time"); + if (event.deathTime() == Time::NEVER) + throw error::Fatal ("Attempt to schedule an Activity without valid deadline"); + Offset toDeadline{now, event.deathTime()}; + if (toDeadline > FUTURE_PLANNING_LIMIT) + throw error::Fatal (util::_Fmt{"Attempt to schedule Activity %s " + "with a deadline by %s into the future"} + % *event.activity + % toDeadline); + } + /** * Decide if Activities shall be performed now and in this thread. * @param when the indicated time of start of the first Activity @@ -224,6 +247,7 @@ namespace gear { if (!event) return activity::SKIP; Time now = executionCtx.getSchedTime(); + sanityCheck (event, now); if (decideDispatchNow (event.startTime(), now)) return ActivityLang::dispatchChain (event, executionCtx); else diff --git a/tests/vault/gear/scheduler-commutator-test.cpp b/tests/vault/gear/scheduler-commutator-test.cpp index be1acc820..dfa15238a 100644 --- a/tests/vault/gear/scheduler-commutator-test.cpp +++ b/tests/vault/gear/scheduler-commutator-test.cpp @@ -112,14 +112,16 @@ namespace test { SchedulerInvocation queue; SchedulerCommutator sched; Activity activity; - Time when{1,2}; + Time when{3,4}; + Time dead{5,6}; // use the ActivityDetector for test instrumentation... ActivityDetector detector; Time now = detector.executionCtx.getSchedTime(); + CHECK (now < dead); // prepare scenario: some activity is enqueued - queue.instruct ({activity, when}); + queue.instruct ({activity, when, dead}); sched.postDispatch (sched.findWork(queue,now), detector.executionCtx,queue); CHECK (detector.verifyInvocation("CTX-tick").arg(now)); @@ -438,7 +440,8 @@ namespace test { // rigged execution environment to detect activations-------------- ActivityDetector detector; Activity& activity = detector.buildActivationProbe ("testActivity"); - + auto makeEvent = [&](Time start) { return ActivationEvent{activity, start, start+Time{0,1}}; }; + // set a dummy deadline to pass the sanity check SchedulerInvocation queue; SchedulerCommutator sched; @@ -456,14 +459,14 @@ namespace test { CHECK (not sched.holdsGroomingToken (myself)); // Activity immediately dispatched when on time and GroomingToken can be acquired - CHECK (activity::PASS == sched.postDispatch (ActivationEvent{activity, past}, detector.executionCtx, queue)); + CHECK (activity::PASS == sched.postDispatch (makeEvent(past), detector.executionCtx, queue)); CHECK (detector.verifyInvocation("testActivity").timeArg(now)); // was invoked immediately CHECK ( sched.holdsGroomingToken (myself)); CHECK ( queue.empty()); detector.incrementSeq(); // Seq-point-1 in the detector log // future Activity is enqueued by short-circuit directly into the PriorityQueue if possible - CHECK (activity::PASS == sched.postDispatch (ActivationEvent{activity, future}, detector.executionCtx, queue)); + CHECK (activity::PASS == sched.postDispatch (makeEvent(future), detector.executionCtx, queue)); CHECK ( sched.holdsGroomingToken (myself)); CHECK (not queue.empty()); CHECK (isSameObject (activity, *queue.peekHead())); // appears at Head, implying it's in Priority-Queue @@ -474,14 +477,14 @@ namespace test { CHECK (queue.empty()); // ...but GroomingToken is not acquired explicitly; Activity is just placed into the Instruct-Queue - CHECK (activity::PASS == sched.postDispatch (ActivationEvent{activity, future}, detector.executionCtx, queue)); + CHECK (activity::PASS == sched.postDispatch (makeEvent(future), detector.executionCtx, queue)); CHECK (not sched.holdsGroomingToken (myself)); CHECK (not queue.peekHead()); // not appearing at Head this time, CHECK (not queue.empty()); // rather waiting in the Instruct-Queue blockGroomingToken(sched); - CHECK (activity::PASS == sched.postDispatch (ActivationEvent{activity, now}, detector.executionCtx, queue)); + CHECK (activity::PASS == sched.postDispatch (makeEvent(now), detector.executionCtx, queue)); CHECK (not sched.holdsGroomingToken (myself)); CHECK (not queue.peekHead()); // was enqueued, not executed diff --git a/tests/vault/gear/scheduler-service-test.cpp b/tests/vault/gear/scheduler-service-test.cpp index a7b88867d..2201020e2 100644 --- a/tests/vault/gear/scheduler-service-test.cpp +++ b/tests/vault/gear/scheduler-service-test.cpp @@ -111,7 +111,7 @@ namespace test { void postNewTask (Scheduler& scheduler, Activity& chain, Time start) { - ActivationEvent actEvent{chain, start}; + ActivationEvent actEvent{chain, start, start + Time{50,0}}; // add dummy deadline +50ms Scheduler::ExecutionCtx ctx{scheduler, actEvent}; scheduler.layer2_.postDispatch (actEvent, ctx, scheduler.layer1_); } diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 8b0e02b68..28a2e86cd 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -82080,21 +82080,74 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
- + - + + + - + - - + + + + +

+ Nicht am falschen Ende sparen hier! Das kostet nur ein paar Nanosekunden... + (und ein paar Fixes in den Tests, damit die am sanityCheck vorbeikommen) +

+ + + + + + + + + + + + + + + + + + + + + + + +

+ 20sec ⟹ 366 MiB +

+ + + + + + + + + +

+ also auf 20 Sekunden in die Zukunft limitieren +

+ + + - - + + + + + + @@ -82104,14 +82157,14 @@ Date:   Thu Apr 20 18:53:17 2023 +0200
 - - + + - - + + - - + +